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

Track transcript SwingStore start position and have upgrade reset to end rather than beginning #3293

Closed
FUDCo opened this issue Jun 11, 2021 · 5 comments · Fixed by #5297
Closed
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@FUDCo
Copy link
Contributor

FUDCo commented Jun 11, 2021

(Note: this task has been redefined, see #3293 (comment) below. We are retaining the issue with its original summary for continuity, but the work to be done here is rather different from what was originally outlined.)

What is the Problem Being Solved?

The SwingStore's stream API provides read and write operations but not delete. Delete is not currently needed, but we will need it if we wish to retire a vat completely and reclaim its resources.

Description of the Design

The proposal here is to augment the StreamStore API with a deleteStream(streamName) method, which will operate in a manner similar to the existing closeStream call but which will also delete the associated stream file.

Security Considerations

I do not believe there are any particular security considerations here above and beyond those posed by access to the StreamStore API in the first place. Obviously there will remain policy questions about whether or not deleting a stream in any particular case is a good idea or not, in particular when we think keeping old streams around may be useful as a diagnostic aid. However, that is a pragmatic consideration rather than a security consideration per se.

Test Plan

Augment the existing StreamStore API tests to exercise this operation and verify that the associated stream file is, in fact, gone after its use.

@FUDCo FUDCo added enhancement New feature or request SwingSet package: SwingSet labels Jun 11, 2021
@FUDCo FUDCo self-assigned this Jun 11, 2021
@FUDCo
Copy link
Contributor Author

FUDCo commented Aug 24, 2021

I think in retrospect that we do not need this, and we can close this issue. @warner ?

@FUDCo
Copy link
Contributor Author

FUDCo commented Nov 12, 2021

After discussion, Brian and I concluded that we actually do need this, though it's relatively low priority. Moving back onto the backlog.

@Tartuffo Tartuffo added MN-1 and removed MN-1 labels Feb 2, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@Tartuffo
Copy link
Contributor

Let's revisit whether we need this for MN-1, or if this is subsumed by other work that @warner is doing.

@Tartuffo Tartuffo removed this from the Mainnet 1 milestone Apr 5, 2022
@FUDCo
Copy link
Contributor Author

FUDCo commented Apr 29, 2022

Redefine this task in terms of keeping track of the historical state, possibly archiving the historical transcript prior to upgrade.

@FUDCo FUDCo added this to the Mainnet 1 milestone Apr 29, 2022
@FUDCo
Copy link
Contributor Author

FUDCo commented Apr 30, 2022

@warner and I discussed this and concluded that the task needs doing but needs to be reconceptualized a bit.

When a vat is upgraded, the transcript of its pre-upgrade life cannot safely be replayed by the post-upgrade version of its code. So upgrade was discarding the old transcript by resetting the lastSnapshot pointer to 0, resulting in the start of a new transcript that began overwriting the old one. This is problematic for a couple of reasons. First, although replays of the new transcript terminate at whatever position endPosition indicates, and thus work correctly, residual transcript data from the pre-upgrade world will still be present in the stream store that holds the transcript from the current end position up to the point where the upgrade happened. This leftover junk is potentially confusing to anyone trying to diagnose problems by inspection of the contents of the stream store. This could be addressed by actually deleting the stream store (then creating a new one) as part of the upgrade operation, as this ticket originally conceived of how things would work. However, a second problem is that by losing the older transcript data (whether by deletion or by overwriting), we make historical replay impossible, and thus preclude later employment of the sleeper agent upgrade protocol should the need arise at a future time.

Instead, we have decided that the correct strategy is to continue appending to the existing transcript, but keep track of the upgrade point so that replays will start from there instead of from 0 when there are no snapshots to begin replaying from. This motivates the addition of a startPosition record alongside the existing endPosition record. Instead of resetting endPosition to 0 on upgrade, startPosition would be set to endPosition and endPosition would be left unchanged.

If the disk space consumed by the growing transcript becomes an issue, at a future time we might add an operation to copy the historical portions of the transcript to an alternative (presumably cheaper) storage medium and truncate the leading portions of the active stream store accordingly. Depending on how replay from archival storage is implemented, this archiving might be entirely orthogonal to the startPosition modifications done by upgrades.

@FUDCo FUDCo changed the title SwingStore stream API needs a delete operation Track transcript SwingStore start position and have upgrade reset to end rather than beginning Apr 30, 2022
@Tartuffo Tartuffo removed this from the Mainnet 1 milestone May 2, 2022
FUDCo added a commit that referenced this issue May 5, 2022
FUDCo added a commit that referenced this issue May 5, 2022
FUDCo added a commit that referenced this issue May 5, 2022
@mergify mergify bot closed this as completed in #5297 May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants