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

Move Rocksdb back to upstream #1196

Merged
merged 4 commits into from
Feb 16, 2024
Merged

Move Rocksdb back to upstream #1196

merged 4 commits into from
Feb 16, 2024

Conversation

AhmedSoliman
Copy link
Contributor

@AhmedSoliman AhmedSoliman commented Feb 15, 2024

Move Rocksdb back to upstream

Rocksdb 0.22 have been released and it includes the statistics changes from rust-rocksdb/rust-rocksdb#853 and rust-rocksdb/rust-rocksdb#854
we can now move back to upstream.


Stack created with Sapling. Best reviewed with ReviewStack.

This was referenced Feb 15, 2024
Copy link

github-actions bot commented Feb 15, 2024

Test Results

111 files  ±0  111 suites  ±0   11m 29s ⏱️ + 1m 21s
100 tests ±0  100 ✅ ±0  0 💤 ±0  0 ❌ ±0 
255 runs  ±0  255 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 8ae017f. ± Comparison against base commit 48cf612.

♻️ This comment has been updated with latest results.

@AhmedSoliman AhmedSoliman changed the title rocksdb back to upstream Move Rocksdb back to upstream Feb 16, 2024
@AhmedSoliman AhmedSoliman marked this pull request as ready for review February 16, 2024 09:47
This introduces a central system to manage long-running and background restate async tasks.
The core of this proposal is to help us lean more towards spawning self-contained tasks that
are addressable, trackable, and cancellable, than current deep future poll trees.

It also allows nice possibilities like:
- Limited structured concurrency (no auto-waiting for children though)
- Graceful cancellations/shutdown reduces the risk of cancellation-unsafe drops
- Potentially less memory, deep nested future state machines become shallower.
- A single place where we can auto-propagate tracing context, flag tasks by priority, schedule tasks on different runtime, provide observability of what kind of tasks are running, etc.
- Scoped tasks allow scoped cancellation. (cooperatively cancel all tasks for a partition id, or a specific tenant in the future, or be specific and filter specific kinds of tasks)
- Limit concurrency of certain tasks by kind, partition, or tenant, etc.
- Distributing tasks among multiple tokio runtimes based on the task kind
- Support for different abort-policy based on the task kind.

A the moment, I only migrated a small pieces of our code to this system to make the merging with #1180 easier.
Once #1180 is merged, I'll move the rest of our services and bifrost to use it.
Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

LGTM. +1 for merging.

This migrates _most_ of the long-running tasks to task center, and we get to harvest the fruit of this investment.
Note that this is still not 100% of the migration, but it's a sufficient start to witness the value.

A good follow-up would be to adopt TaskCenter's potential of scheduling tasks on multiple runtimes and move the rocksdb
single-threaded writer runtime to be managed by TaskCenter as well, but that's for another time.
s/my_node_node/my_node_id/
Rocksdb 0.22 have been released and it includes the statistics changes from rust-rocksdb/rust-rocksdb#853 and rust-rocksdb/rust-rocksdb#854
we can now move back to upstream.
@AhmedSoliman AhmedSoliman merged commit 37b88e7 into main Feb 16, 2024
8 checks passed
@AhmedSoliman AhmedSoliman deleted the pr1196 branch February 16, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants