Skip to content

Conversation

@ARR4N
Copy link
Collaborator

@ARR4N ARR4N commented Nov 19, 2025

Closes #29, an open question after merging the saexec.Executor.

@StephenButtolph
Copy link
Contributor

StephenButtolph commented Nov 19, 2025

I'm not sure that this closes #29. Or perhaps I had really wanted a more general issue. The comment I made on the executor PR was intended to be a bit more holistic than just the snapshot logic.

Specifically, (after talking with @alarso16). I think we need to be calling triedb.Commit to flush the trie nodes to disk during shutdown. Additionally, I think we will want to periodically be committing the triedb during execution to limit the amount of reexecution that is required upon a restart (in the case of an ungraceful shutdown).

@ARR4N
Copy link
Collaborator Author

ARR4N commented Nov 20, 2025

I'm not sure that this closes #29. Or perhaps I had really wanted a more general issue. The comment I made on the executor PR was intended to be a bit more holistic than just the snapshot logic.

Ahh your DM said snapshot.

Specifically, (after talking with @alarso16). I think we need to be calling triedb.Commit to flush the trie nodes to disk during shutdown. Additionally, I think we will want to periodically be committing the triedb during execution to limit the amount of reexecution that is required upon a restart (in the case of an ungraceful shutdown).

Yup, we are absolutely not persisting everything to disk yet. If you look in the PoC that's done in the maybeCommitTrieDB() function. It may turn out that we move that into the Executor, but it definitely wasn't my intention to include it in the last PR (hence your confusion).

@ARR4N ARR4N merged commit 50f3f50 into main Nov 20, 2025
11 checks passed
@ARR4N ARR4N deleted the arr4n/demonstrate-snapshot-persistence branch November 20, 2025 15:55
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.

Test persistence of snapshot to disk on saexec.Executor.Close()

3 participants