Skip to content

Remove outdated table slice encodings #2798

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

Conversation

dominiklohmann
Copy link
Member

Following the introduction of a minimum partition version of 1 with #2778, we're no longer able to encounter any table slices that are MessagePack-encoded or use an older version of the mapping of our data model onto Apache Arrow.

There's a future refactoring that is now unblocked, which is a total replacement of the legacy baggage that is the table_slice API with an easier-to-use wrapper around the same underlying data that makes more assumptions about the underlying column-major memory layout.

This is based on #2797 to avoid merge conflicts.

patszt and others added 4 commits December 14, 2022 15:05
Following the introduction of a minimum partition version of 1 with
#2778, we're no longer able to encounter any table slices
that are MessagePack-encoded or use an older version of the mapping of
our data model onto Apache Arrow.

There's a future refactoring that is now unblocked, which is a total
replacement of the legacy baggage that is the `table_slice` API with an
easier-to-use wrapper around the same underlying data that makes more
assumptions about the underlying column-major memory layout.

This is based on #2797 to avoid merge conflicts.
@dominiklohmann dominiklohmann added the maintenance Tasks for keeping up the infrastructure label Dec 14, 2022
@dominiklohmann dominiklohmann requested a review from a team December 14, 2022 23:57
dominiklohmann and others added 2 commits December 15, 2022 01:24
We've removed the Broker plugin—it was written for Zeek 3.x and never
worked with newer versions.
It's dependencies were also troublesome to get them working with CAF
0.18.
The real-time Zeek import will return in a future plugin by connecting
to Zeek directly via a websocket
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.

The changes look good. Can you convince the reviewer that they are complete?

@dominiklohmann
Copy link
Member Author

@mavam
Can you convince the reviewer that they are complete?

The process was the following:

  • Remove the four outdated entries in the FlatBuffers table without changing the index of the current one; I manually tested this for backwards compatibility.
  • Remove code that tried to access the now removed entries in the code that flatc generated.
  • Remove all code related to MessagePack.
  • Remove all deprecated functions related to Arrow.
  • Remove everything relating to the upgrade path from older Arrow encodings to the most recent one.

The one thing table_slice did right was hiding the underlying FlatBuffers table, which was only ever included in a single file, namely table_slice.cpp. This made it rather trivial to do remove the old encodings.

@dominiklohmann dominiklohmann removed the request for review from a team December 16, 2022 00:24
@dominiklohmann dominiklohmann merged commit eed23f9 into story/sc-39998/remove-the-archive Dec 16, 2022
@dominiklohmann dominiklohmann deleted the story/sc-39999/remove-old-table-slice-encodings branch December 16, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Tasks for keeping up the infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants