feat: add rw_rate_limit system catalog #19466
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Nov 20, 2024
This stack of pull requests is managed by Graphite. Learn more about stacking. |
github-actions
bot
added
type/fix
xxchan
changed the title
xxchan
changed the title
xxchan
force-pushed
the
11-16-feat_support_rw_rate_limit
branch
2 times, most recently
from
8e28432 to
425261d
Compare
xxchan
commented
Nov 20, 2024
Comment on lines
24
to
25
| #[primary_key] | ||
| fragment_id: i32, |
There was a problem hiding this comment.
Maybe in theory 1 fragment can have multiple rate-limited nodes and it's not primary key
|
Oops. Seems this system catalog is not isolated between databases 🤔 
Some catalogs support this, but some don't: e.g,
|
BugenZhao
approved these changes
Nov 20, 2024
System catalogs about metadata of the streaming job are all not isolated between databases. They are used solely for debugging. |
|
Yes, I was also a little hesitant to add isolation which might hurt debugging. 🤪 |
yezizp2012
approved these changes
Nov 20, 2024
| fn bitflag_intersects(column: SimpleExpr, value: i32) -> SimpleExpr { | ||
| column | ||
| .binary(BinOper::Custom("&"), value) | ||
| .binary(BinOper::NotEqual, 0) |
There was a problem hiding this comment.
👍 Wow, it can actually be written like this. Does it work for PostgreSQL and MySQL backends? I'm not sure if these slts are covered by e2e tests for other backends. Cc @BugenZhao
There was a problem hiding this comment.
I checked their doc and tested locally and it works fine.
But seems not covered in CI.
Merge activity
|
xxchan
force-pushed
the
11-16-feat_support_rw_rate_limit
branch
from
40227bb to
6f13304
Compare
9 tasks
xxchan
force-pushed
the
11-16-feat_support_backfill_rate_limit_for_source_backfill
branch
from
3993c63 to
a7a89ea
Compare
xxchan
changed the base branch from
11-16-feat_support_backfill_rate_limit_for_source_backfill
to
graphite-base/19466
feat: support rw_rate_limit refine Signed-off-by: xxchan <[email protected]> fix primary key Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
xxchan
force-pushed
the
graphite-base/19466
branch
from
a7a89ea to
fe69ce3
Compare
xxchan
force-pushed
the
11-16-feat_support_rw_rate_limit
branch
from
561e063 to
ebc55b3
Compare
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 20, 2024
…19466) (#19482) Signed-off-by: xxchan <[email protected]> Co-authored-by: Noel Kwan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
It's not easy to know what jobs are rate limited, so add this.
I want to avoid iterating over all fragments, so I introduced a new fragment flag for FsFetch. This means
rw_rate_limitwon't show fs fragments created before this PR, and I think it's acceptable.Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.