Skip to content

[#1025] Stop a restore which cannot lock its backend instead of completing it - #1027

Open
vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issues/1025-restore-lock-failure
Open

[#1025] Stop a restore which cannot lock its backend instead of completing it#1027
vharseko wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:issues/1025-restore-lock-failure

Conversation

@vharseko

@vharseko vharseko commented Sep 11, 2026

Copy link
Copy Markdown
Member

Fixes #1025.

Rebased onto master after #969 (e86f702f8e) landed; the branch is the single commit 3e600a73a1, its content unchanged from the original ad67ade1e9 (same patch-id).

The bug

RestoreTask.runTask() guarded the restore with if (verifyOnly || lockBackend(backend)) and had no else. When the backend lock was refused - another process holds it, or the lock file cannot be created - the whole restore block was skipped, errorsEncountered stayed false, the finally re-enabled the backend and told the listeners the restore succeeded, and the task returned getFinalTaskState(), i.e. COMPLETED_SUCCESSFULLY. The restore CLI printed the ERR_RESTOREDB_CANNOT_LOCK_BACKEND line from the task log and exited 0 right after it.

The change

A refused lock ends the task the way BackupTask and ImportTask already end theirs, and the way #969 already ends a failed disableBackend():

if (!verifyOnly && !lockBackend(backend))
{
  errorsEncountered = true;
  return TaskState.STOPPED_BY_ERROR;
}

The return passes through the finally, which re-enables the backend and notifies processRestoreEnd(..., false). The rest of the RestoreTask hunk is the de-indentation of the former if body - ?w=1 shows the five real lines.

restore --verifyOnly is unaffected: it never takes the lock.

Test

TestBackupAndRestore.testRestoreEndsInErrorWhenTheBackendCannotBeLocked - self-contained: it backs userRoot up into a temporary directory, takes a shared lock on the backend's lock file and restores from that backup. LockFileManager reference-counts shared locks, so the task's disableBackend() releases the backend's own reference but not the test's, and the exclusive lock the restore needs is refused with ERR_FILELOCKER_LOCK_EXCLUSIVE_REJECTED_BY_SHARED. Asserted: STOPPED_BY_ERROR, one begin and one end notification, successful == false at the end notification, and userRoot registered again afterwards.

Before the change the test fails with expected [STOPPED_BY_ERROR] but found [COMPLETED_SUCCESSFULLY], the task log carrying the refused-lock error. It also runs the restore half of #969's finally - the backendDisabled re-enable and the !errorsEncountered notification - which the review of #969 noted no test executed.

Run: TestBackupAndRestore 13/13, TestImportAndExport 14/14, PrivilegeTestCase 185/185, ReSyncTest 2/2, LDIFBackendTestCase 22/22.

@vharseko vharseko added bug tests Test suites: fixing, enabling, un-disabling labels Sep 11, 2026
…end instead of completing it

RestoreTask guarded the restore with `if (verifyOnly || lockBackend(backend))`
and had no else branch: when the backend lock was refused the whole restore
block was skipped, errorsEncountered stayed false, the backend was re-enabled,
the listeners were told the restore succeeded and the task ended in
COMPLETED_SUCCESSFULLY - the restore CLI exited 0 right after printing the
lock error.

A refused lock now ends the task in STOPPED_BY_ERROR through the finally which
re-enables the backend and reports the failure to the listeners, the way
BackupTask and ImportTask already treat it.

Fixes OpenIdentityPlatform#1025.
@vharseko
vharseko force-pushed the issues/1025-restore-lock-failure branch from ad67ade to 3e600a7 Compare September 11, 2026 13:56
@vharseko

Copy link
Copy Markdown
Member Author

@maximthomas rebased onto master now that #969 has landed - the merge conflict is gone. The branch is the single commit 3e600a73a1, the same patch as ad67ade1e9 (identical patch-id), so there is nothing new to review beyond that one commit. The description no longer mentions the stack.

@vharseko
vharseko requested review from maximthomas and removed request for maximthomas September 11, 2026 14:12
@vharseko vharseko added tasks Server administrative tasks: import, export, backup, restore data-loss Data integrity / loss of entries labels Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug data-loss Data integrity / loss of entries tasks Server administrative tasks: import, export, backup, restore tests Test suites: fixing, enabling, un-disabling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A restore task that cannot lock its backend skips the restore and completes successfully

1 participant