-
Notifications
You must be signed in to change notification settings - Fork 600
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(risedev): support 3 compute nodes in playground #4332
Conversation
Signed-off-by: Bugen Zhao <[email protected]>
Need to add compactor? |
Both LGTM. |
Signed-off-by: Bugen Zhao <[email protected]>
src/object_store/src/object/mem.rs
Outdated
} | ||
} | ||
|
||
/// Create a shared reference to the in-memory object store in this process. Used for multiple | ||
/// compute-nodes or compactors in the same process by `risedev playground`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add a feature in the crate to enable this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function should not be used by default, so SHARED
should not be initialized and there's no side effect. Adding a feature for this may make playground deployment complicated. 🥵
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added explicit notes for this and make it private outside the super module.
Codecov Report
@@ Coverage Diff @@
## main #4332 +/- ##
==========================================
- Coverage 74.32% 74.31% -0.02%
==========================================
Files 844 844
Lines 122474 122496 +22
==========================================
Hits 91027 91027
- Misses 31447 31469 +22
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Also I’m wondering how many compactors are started indeed. For in-memory mode, every compute node will start a compactor. So maybe we have started 4? |
Oops, seems to be a problem. What's the optimal solution? How about disabling compactor in compute node under shared in-memory mode? |
Can we make compactor to start with object store instance in in-memory mode? |
Currently it does, so we've started 4. 🤣 Let's do not start compactor in shared in-memory mode, behaving like minio/s3? |
Looks good. So we'll need a flag to tell each node whether to enable compactor or not. Note that we also have another edge case. When |
…4332) * feat(risedev): support 3 compute nodes in playground Signed-off-by: Bugen Zhao <[email protected]> * Update src/risedevtool/src/task/utils.rs * add compactor Signed-off-by: Bugen Zhao <[email protected]> * add notes Signed-off-by: Bugen Zhao <[email protected]> * make it private outside the super module Signed-off-by: Bugen Zhao <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Signed-off-by: Bugen Zhao [email protected]
I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.
What's changed and what's your intention?
As title. Used
lazy_static
for shared in-memory object store.To start this:
Checklist
./risedev check
(or alias,./risedev c
)Refer to a related PR or issue link (optional)
Close #4195.