Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve continuous export failure recovery by moving the export operation "progress marker" from USB drive to VxScan store #4213

Merged
merged 5 commits into from
Nov 15, 2023

Conversation

arsalansufi
Copy link
Contributor

@arsalansufi arsalansufi commented Nov 15, 2023

Easiest reviewed by commit

Overview

Issue link: #4096

This PR improves continuous export failure recovery by moving the export operation "progress marker" from the USB drive to the VxScan store.

We currently write a .vx-export-in-progress file to the USB drive at the start of every continuous export operation. We then delete the file when the operation completes. If an export operation fails midway and a machine has to be restarted, the existence of that file on startup tells us that a resync is required.

The main downside to using a file on the USB drive is that the write of that file itself may fail. In that case, the machine won't know that a resync is required after restart. Switching to a piece of state on the machine is more robust, especially when paired with a transaction.

Testing

  • Verified that existing tests still pass
  • Added unit tests
  • Tested failure recovery manually by removing the USB drive right after scanning a ballot

Checklist

  • I have added logging where appropriate to any new user actions, system updates such as file reads or storage writes, or errors introduced N/A

@arsalansufi arsalansufi force-pushed the arsalan/ce-failure-recovery-refinements branch from 51f3cf9 to 131953a Compare November 15, 2023 15:01
// If we're storing an accepted sheet that needed review, that means that it was "adjudicated"
// (i.e. the voter said to count it without changing anything).
if (interpretation.type === 'NeedsReviewSheet') {
store.adjudicateSheet(sheetId);
}

// Gets reset to false within exportCastVoteRecordsToUsbDrive
store.setIsContinuousExportOperationInProgress(true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually implement this store method in the next commit

setExportDirectoryName(exportDirectoryName: string): void;
setIsContinuousExportOperationInProgress(
isContinuousExportOperationInProgress: boolean
): void;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than having all these methods be optional on the ScannerStore interface, I've now updated ScannerStore to be a union of two distinct interfaces, one for the central scanner and one for the precinct scanner (still sharing what I can though, via ScannerStoreBase)

@arsalansufi arsalansufi requested a review from adghayes November 15, 2023 15:25
@arsalansufi arsalansufi marked this pull request as ready for review November 15, 2023 15:25
from is_continuous_export_operation_in_progress
`
) as { isContinuousExportOperationInProgress: number } | undefined;
return Boolean(row?.isContinuousExportOperationInProgress);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added a type + basic utils to admin/backend/src/store.ts for being more explicit about these boolean conversions. What do you think about copying those or moving them to a shared library to use in this store file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice! Can move to a shared location

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a9caa14

@arsalansufi arsalansufi merged commit a45b064 into main Nov 15, 2023
@arsalansufi arsalansufi deleted the arsalan/ce-failure-recovery-refinements branch November 15, 2023 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants