Skip to content
This repository has been archived by the owner on Feb 17, 2024. It is now read-only.

timemachine: support seeking through log records #45

Merged
merged 2 commits into from
Jun 1, 2023
Merged

Conversation

achille-roussel
Copy link
Contributor

Following up from conversations with @pelletier and @chriso, and in relation with #43, this PR adds the ability to seek through the log records by implementing io.Seeker.

@achille-roussel
Copy link
Contributor Author

The implementation is rudimentary, there are worst-case scenarios like stepping back to the previous offset repeatedly would cause a full scan from the start each time; we can figure out optimizations later.

I also need to add tests, but I think I want to first modify the timemachine.Record type to not be backed by a flatbuffers because it makes it too difficult to construct records to test scenarios.

@@ -120,6 +157,11 @@ func (r *LogReader) readFrame() (*buffer.Buffer, error) {
return f, nil
}

var (
_ io.Closer = (*LogReader)(nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why not?

Suggested change
_ io.Closer = (*LogReader)(nil)
_ io.ReadCloser = (*LogReader)(nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we can't read bytes from the LogReader, the underlying io.Reader isn't exposed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry i meant _ io.ReadSeeker = (*LogReader)(nil) (combining this check and the one below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah we could have combined them, same same 😁

Co-authored-by: Thomas Pelletier <[email protected]>
@achille-roussel achille-roussel merged commit d2ba046 into main Jun 1, 2023
3 checks passed
@achille-roussel achille-roussel deleted the seek branch June 1, 2023 19:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants