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

feat: support per table memtable options #3524

Merged
merged 15 commits into from
Mar 19, 2024

Conversation

evenyag
Copy link
Contributor

@evenyag evenyag commented Mar 15, 2024

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This PR supports configuring memtable options for each table. It replaces the global memtable builder in the worker with a builder factory MemtableBuilderProvider.

By default, all regions use the same default_memtable_builder. If memtable.type is specified in the region options, the provider will create a new memtable for the region. The memtable config in the MitoConfig only applies to the default memtable.

create table if not exists test_memtable(
    host string,
    ts timestamp,
    memory double,
    TIME INDEX (ts),
    PRIMARY KEY(host)
)
engine=mito
with(
    'memtable.type' = 'time_series',
);

This PR changes the default global memtable to the old time_series memtable. It enables the new memtable in the metric engine by setting the memtable type explicitly.

It also fixes an issue that the region uses the default value instead of returning an error if compaction.twcs.max_active_window_files isn't a valid integer.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR does not require documentation updates.

@github-actions github-actions bot added the docs-not-required This change does not impact docs. label Mar 15, 2024
@evenyag evenyag force-pushed the feat/region-memtable-builder branch from 54ab0d8 to 4684242 Compare March 15, 2024 08:16
@evenyag evenyag force-pushed the feat/region-memtable-builder branch from c26b108 to 90bb352 Compare March 18, 2024 09:20
@evenyag evenyag marked this pull request as ready for review March 18, 2024 11:48
Copy link
Contributor

@v0y4g3r v0y4g3r left a comment

Choose a reason for hiding this comment

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

LGTM

@v0y4g3r
Copy link
Contributor

v0y4g3r commented Mar 19, 2024

Hopefully we won't end up with MemtableBuilderProviderBuilder 😅

@v0y4g3r v0y4g3r added this pull request to the merge queue Mar 19, 2024
Merged via the queue into GreptimeTeam:main with commit 6415926 Mar 19, 2024
32 checks passed
@v0y4g3r v0y4g3r deleted the feat/region-memtable-builder branch March 19, 2024 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required This change does not impact docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants