Skip to content

Use just a single option for setting the table slice type #951

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 24, 2020
Merged

Use just a single option for setting the table slice type #951

merged 4 commits into from
Jun 24, 2020

Conversation

dominiklohmann
Copy link
Member

We have a table-slice-type option available as both system.table-slice-type and import.table-slice-type. This is quite confusing. Let's make it a single option just like import.table-slice-type.

@dominiklohmann dominiklohmann requested a review from a team June 24, 2020 10:10
@lava
Copy link
Member

lava commented Jun 24, 2020

Are the table slice type/size constants only used during import? If not, system may be the better name to keep since it will be confusing to have e.g. export-related options in import.table_slice_size.

Also, I wonder what the current semantics are if we import with a different table slice type/size than the one configured in the node? Would this result in an error?

@dominiklohmann
Copy link
Member Author

Are the table slice type/size constants only used during import? If not, system may be the better name to keep since it will be confusing to have e.g. export-related options in import.table_slice_size.

Only during ingestion, which means during import and when adding vast.metrics events.

Also, I wonder what the current semantics are if we import with a different table slice type/size than the one configured in the node? Would this result in an error?

For the type this never mattered, for the size this was fixed by #908.

lava
lava requested changes Jun 24, 2020
View reviewed changes
Copy link
Member

@lava lava left a comment

Choose a reason for hiding this comment

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

The code change itself looks good to me, only thing I'm wondering is the msgpack-related question below.

@dominiklohmann dominiklohmann requested a review from lava June 24, 2020 12:04
@dominiklohmann
Use a single option for table-slice-type
af71e2c 
This change makes `import.table-slice-type` the only option for setting
the desired table slice type. `system.table-slice-type` does no longer
exist.
@dominiklohmann
Use a single option for table-slice-size
2426fc0 
This was changed previously, but this one line snuck through the review.
@dominiklohmann
Add changelog entry
3350f9f
@dominiklohmann dominiklohmann merged commit 9d8a3fb into master Jun 24, 2020
@dominiklohmann dominiklohmann deleted the story/ch15456 branch June 24, 2020 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lava lava lava approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dominiklohmann @lava