refactor(optimizer): also use NoOp node to avoid empty fragment
#9381
Conversation
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Codecov Report
@@ Coverage Diff @@
## main #9381 +/- ##
==========================================
- Coverage 70.80% 70.79% -0.01%
==========================================
Files 1227 1226 -1
Lines 203451 203449 -2
==========================================
- Hits 144050 144038 -12
- Misses 59401 59411 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| | └─StreamFilter { predicate: IsNotNull(date_time) } | ||
| | └─StreamRowIdGen { row_id_index: 7 } | ||
| | └─StreamSource { source: "bid", columns: ["auction", "bidder", "price", "channel", "url", "date_time", "extra", "_row_id"] } | ||
| | └─StreamShare { id = 7 } |
There was a problem hiding this comment.
An observation: this plan can be further optimized because all parents of the share node are StreamExchange { dist: HashShard(window_start) }. Currently, we need to shuffle twice, but one of them is wasted.
There was a problem hiding this comment.
Sounds we need a better StreamShare::to_stream()
There was a problem hiding this comment.
Yes, to remove this shuffle, we need to collect some information before StreamShare::to_stream().
There was a problem hiding this comment.
First I guess we can make all Share transform to NoShuffle exchange, but there seem to be some bugs in scaling now. I'll investigate it.
Signed-off-by: Bugen Zhao i@bugenzhao.comI hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR is a sequel to #9320. As we do not support empty fragments in the meta service, and
Sharewill lead to empty fragment sometimes, we used to apply an optimizer rule in #8249 to prevent this from happening.As suggested by @st1page, this PR further handle this in the logic of fragmenter, so that we don't have to worry about any other cases of consecutive
Exchangeas well (though it's actually rare). Also, we useNoOpexecutor to avoid the overhead of evaluatingInputRef.Checklist For Contributors
./risedev check(or alias,./risedev c)Checklist For Reviewers
Documentation
Click here for Documentation
Types of user-facing changes
Please keep the types that apply to your changes, and remove the others.
Release note