Skip to content

Use Flatbuffers for Persistent State of Segment Store and Meta Index #972

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

Merged
merged 70 commits into from
Jul 13, 2020

Conversation

lava
Copy link
Member

@lava lava commented Jul 13, 2020

We want to use a well-defined on-disk format for VAST, to enable stable upgrade paths for VAST users.

This PR adds the first step of this effort by changing the segment store and the meta index to use a flatbuffer layout for its persistent on-disk data.

In addition, some preparatory work for more complete flatbuffer integration in the future is included, such as the Layout and Type type definitions.

Finally, a new filesystem actor is included, which is not strictly related to flatbufferization but will be used in follow-up work to centralize the file reads and writes when loading and storing flatbuffers.

NOTE: This PR adds a hard dependency on flatbuffers to VAST.

mavam and others added 30 commits May 20, 2020 17:10
The utilities in namespace fbs are meant to facility conversion from
flatbuffers to VAST and vice versa.
Since a segment wraps a reference-counted chunk, it's not necessary to
add another layer of reference counting on top.
Because there is no PPA for version 1.11 for the Ubuntu version of the
github CI.
We added an extra layer of indirection into the segment flatbuffer so
that we can efficiently append table slice flatbuffers to the segment.
See https://groups.google.com/forum/#!topic/flatbuffers/6KRC02iXDhs for
the detailed discussion that motivated this change.
Co-Authored-By: tobim <tobim@fastmail.fm>
Support versioning of persistent data structures
lava and others added 8 commits July 3, 2020 17:17
We observed a race condition in unit tests where the terminator
could sometimes "miss" a down message if one of the actors it
tried to kill was a detached actor.

In this case, the timeout would trigger but the terminator was
left alive even though all the actors it tries to terminate are
gone, leading to an indefinite system hang at shutdown.

We explicitly kill the terminator as a workaround, until a more
exhaustive shutdown logic using retries is implemented or the
caf bug involving detached actors is fixed.
This was changed in master, so we fix the remaining instances
in this branch.
Add filesystem actor
@tobim
Copy link
Member

tobim commented Jul 13, 2020

Can you squash my fixup! commit?

I'm going to check if the branch works with the pro version locally.

@lava lava force-pushed the epic/application-state-versioning branch from c5862ae to e98dc27 Compare July 13, 2020 08:58
@dominiklohmann
Copy link
Member

Should we rebase this onto current master?

@lava
Copy link
Member Author

lava commented Jul 13, 2020

@dominiklohmann The last commit merges current master into the branch.

Rebasing is problematic with such a big branch: First of all it is pretty labor-intensive but more importantly it potentially adds new conflict resolutions to a ton of old commits, potentially introducing subtle bugs that cannot be reviewed and invalidating the existing reviews.

Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

Can you add a changelog entry?

I think I'm fine with this set of changes being merged after the testing we've done.

Copy link
Member

@tobim tobim left a comment

Choose a reason for hiding this comment

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

Please resolve the conflict, other than that I think this can be merged.

Copy link
Member

@mavam mavam left a comment

Choose a reason for hiding this comment

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

Just some changelog smithing from my side.

@lava lava force-pushed the epic/application-state-versioning branch from ab55b53 to cc4cd7c Compare July 13, 2020 14:22
@lava lava force-pushed the epic/application-state-versioning branch from cc4cd7c to 0d8fb9a Compare July 13, 2020 14:31
@tobim tobim requested a review from dominiklohmann July 13, 2020 15:24
@lava lava merged commit 36878eb into master Jul 13, 2020
@lava lava deleted the epic/application-state-versioning branch July 13, 2020 15:50
@dominiklohmann dominiklohmann added the feature New functionality label Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants