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

Implement new MessagePack-encoded table slices #1157

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Nov 9, 2020

📔 Description

The fifth PR to be merged into #1140. This one re-implements MessagePack-encoded table slices to use FlatBuffers directly.

📝 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

🎯 Review Instructions

Commit-by-commit feels best.

@dominiklohmann dominiklohmann added refactoring Restructuring of existing code enhancement ✨ labels Nov 9, 2020
@dominiklohmann dominiklohmann changed the base branch from master to epic/flatbuffers-persistence-table-slices November 9, 2020 22:06
This prepares the `table_slice_builder` for the upcoming changes by
doing some much-needed housecleaning.
Much like the `table_slice_builder`, some cruft accumulated on
`msgpack_table_slice_builder` that needed to be removed.
@dominiklohmann dominiklohmann marked this pull request as ready for review November 10, 2020 10:49
The `table_slice::inspect` function must go through the constructor that
handles the instance counting when loading table slices from a
`caf::message`.
The offset being in the `table_slice` encoding-specific FlatBuffers
tables was the only reason why we required support for mutable
FlatBuffers. This commit changes `table_slice` to hold the offset
in-memory. The offset is now assigned by the importer on import and by
the segment on lookup.
The FlatBuffers API guarantees that the encoding pointers are not
`nullptr`, so we might as well dereference them in the visitor function
already.
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.

Tested locally, feature-wise it works as expected and performance analysis is will be completed on the feature branch.

@dominiklohmann dominiklohmann merged commit 2ef23eb into epic/flatbuffers-persistence-table-slices Nov 12, 2020
@dominiklohmann dominiklohmann deleted the topic/table-slice-msgpack branch November 12, 2020 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Restructuring of existing code
Projects
None yet
2 participants