Simplify partition structure #728

tobim · 2020-01-28T16:46:15Z

This needs to be cleaned and rebased. Do not review yet.

TODO:

Handle pattern matches in type queries
Unit tests
Cleanup partition
Reorganize commits

dominiklohmann

This is looking fine so far. I'm excited for this change, but as you said in-person the export/count side needs to be implemented first to see if this all works out or if you forgot about some component that needs to be touched as well.

I can't see any besides the things we talked about yesterday. So far things are looking fine, but there's still lots of work to do.

libvast_test/src/main.cpp

libvast/vast/system/indexer_stage_driver.hpp

mavam

Overall, the architecture of the change set feels right. Unfortunately there are too many difference concerns and optimizations in one big chunk that it's hard to review accurately. Splitting this up in more digestible pieces would be the next step.

libvast/src/system/index.cpp

libvast/src/system/evaluator.cpp

libvast_test/src/actor_system.cpp

libvast/vast/system/partition.hpp

libvast/vast/column_index.hpp

libvast/src/system/partition.cpp

mavam

This is getting close. At this point I have mainly refactoring comments.

libvast/src/system/index.cpp

libvast/src/system/indexer_downstream_manager.cpp

libvast/vast/qualified_record_field.hpp

libvast/vast/system/index.hpp

libvast/vast/system/indexer_stage_driver.hpp

libvast/src/qualified_record_field.cpp

CHANGELOG.md

libvast/src/system/index.cpp

libvast/src/system/indexer_downstream_manager.cpp

libvast/src/system/spawn_index.cpp

libvast/vast/system/instrumentation.hpp

tobim · 2020-04-21T08:45:08Z

The recent round of comments have been addressed or replied to.

mavam

Almost there!

libvast/src/system/index.cpp

libvast/vast/table_slice_column.hpp

libvast/test/system/indexer.cpp

Prior to this commit, a partition was a container for a set of tables that each directly corresponded to one layout as imported into VAST. This commit modifies the index to decompose table slices into columns at the boundary and promotes the indexer actors responsible for each such column into a member of the partition. The effect is twofold: - An updated schema will not result in a duplication of all columns that exist in both versions of a type with the same name, but only the ones that are actually different. - The filesystem structure is consolidated in the same way, which results in slightly less reads when a query hits a persisted partition, and it is easier to inspect the the disk space usage of the individual fields of the input.

Co-Authored-By: Matthias Vallentin <matthias@tenzir.com>

tobim added the refactoring Restructuring of existing code label Jan 28, 2020

mavam changed the title ~~Simplify partition structure.~~ Simplify partition structure Jan 28, 2020

tobim force-pushed the story/ch9680 branch 2 times, most recently from 3a481df to 96de62f Compare January 29, 2020 20:58

tobim force-pushed the story/ch9680 branch from 96de62f to 98dc273 Compare March 23, 2020 14:59

dominiklohmann self-requested a review March 23, 2020 15:44

dominiklohmann reviewed Mar 24, 2020

View reviewed changes

libvast_test/src/main.cpp Outdated Show resolved Hide resolved

tobim force-pushed the story/ch9680 branch 3 times, most recently from 8906a41 to 6c6d6cf Compare April 6, 2020 22:06

tobim force-pushed the story/ch9680 branch from 6c6d6cf to 4d72d73 Compare April 9, 2020 09:00

mavam reviewed Apr 9, 2020

View reviewed changes

libvast/vast/system/indexer_stage_driver.hpp Outdated Show resolved Hide resolved

mavam reviewed Apr 9, 2020

View reviewed changes

libvast/vast/system/indexer_stage_driver.hpp Show resolved Hide resolved

mavam requested changes Apr 15, 2020

View reviewed changes

tobim force-pushed the story/ch9680 branch 3 times, most recently from a698e19 to fb587c3 Compare April 17, 2020 15:08

tobim marked this pull request as ready for review April 17, 2020 15:10

mavam requested changes Apr 18, 2020

View reviewed changes

tobim force-pushed the story/ch9680 branch 2 times, most recently from 00e29b6 to 5e549b1 Compare April 20, 2020 14:23

mavam reviewed Apr 20, 2020

View reviewed changes

libvast/src/qualified_record_field.cpp Outdated Show resolved Hide resolved

mavam reviewed Apr 21, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

mavam requested changes Apr 21, 2020

View reviewed changes

libvast/src/system/index.cpp Outdated Show resolved Hide resolved

libvast/src/system/indexer_downstream_manager.cpp Show resolved Hide resolved

libvast/src/system/spawn_index.cpp Show resolved Hide resolved

dominiklohmann reviewed Apr 21, 2020

View reviewed changes

libvast/vast/system/instrumentation.hpp Outdated Show resolved Hide resolved

tobim force-pushed the story/ch9680 branch from 03e8ad5 to 4865e94 Compare April 21, 2020 08:32

mavam requested changes Apr 21, 2020

View reviewed changes

libvast/src/system/index.cpp Outdated Show resolved Hide resolved

libvast/vast/table_slice_column.hpp Outdated Show resolved Hide resolved

libvast/test/system/indexer.cpp Outdated Show resolved Hide resolved

mavam approved these changes Apr 21, 2020

View reviewed changes

Enable assertions by default on non-release builds

303fb46

tobim and others added 26 commits April 21, 2020 17:01

Remove table indexer

2f0fb75

Use stable_map to store partition indexers

a853aac

Rename to qualified_record_field

ed99064

Add explanatory comments

0e027c1

Fixup documentation and nameing

ce625fc

Split out table_slice_column

fb76927

Correct the description of the column assignment

b6a99b3

Clear streaming paths for removed partitions

2beed3c

Simplify table slice spreading example

1f30d41

Minor comment and coding style improvements

b547c1e

Clean up API of qualified_record_field

7d16927

Use a stable_map for the pending_query_map

2f0f074

Remove overly verbose comments

fa290e4

Check if partition exists before unregistering

88eef1a

Make sure the capacity does not wrap around

666ef34

Improve indexer paths cleanup logic robustness.

37b130b

Allow passing index options on the command line

9d9a2ba

Force many partitions in the basic counting test

d750463

Clarify buffer size calculation comment

78dbd18

Remove obsolete unit test

94d0800

Abstract away atomic measurement collection

e05bb26

Fine-tune changelog entry for index options

79719ab

Co-Authored-By: Matthias Vallentin <matthias@tenzir.com>

Change to_record_field -> as_record_field

c277bbd

Failing to unregister a partition is a logic error

c547a24

Adhere to coding guidelines

fdbfd0c

Co-Authored-By: Matthias Vallentin <matthias@tenzir.com>

tobim force-pushed the story/ch9680 branch from ec94892 to fdbfd0c Compare April 21, 2020 15:41

tobim merged commit 5890689 into master Apr 21, 2020

tobim deleted the story/ch9680 branch April 21, 2020 21:25

lava mentioned this pull request Apr 23, 2020

Expose VAST_ENABLE_ASSERTIONS as cmake option #838

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify partition structure #728

Simplify partition structure #728

tobim commented Jan 28, 2020 •

edited

Loading

dominiklohmann left a comment

mavam left a comment

mavam left a comment

tobim commented Apr 21, 2020

mavam left a comment

Simplify partition structure #728

Simplify partition structure #728

Conversation

tobim commented Jan 28, 2020 • edited Loading

dominiklohmann left a comment

Choose a reason for hiding this comment

mavam left a comment

Choose a reason for hiding this comment

mavam left a comment

Choose a reason for hiding this comment

tobim commented Apr 21, 2020

mavam left a comment

Choose a reason for hiding this comment

tobim commented Jan 28, 2020 •

edited

Loading