revert #7957 to validate the performance regression of q7 and try to improve if true

[lmatz]

#7957

Meanwhile q7-rewrite which uses group top-n does not change performance much. Didn't see other queries also affected.

[lmatz]

Doubled the throughput after reverting the PR

https://github.com/risingwavelabs/risingwave/tree/lz/revert_7957

[lmatz]

check the performance of the queries that involve join:
https://buildkite.com/risingwave-test/nexmark-benchmark/builds/571#0186de49-a190-42f2-a56f-4b31e4130513

Let's wait and see more results...

http://risingwave-perf-test-dashboard-metabase.us-west-2.elasticbeanstalk.com/question/2633-avg-source-output-rows-per-second-rows-s-history-thtb-119?start_date=2023-02-08

[lmatz]

Probably it still depends on the real workload to determine whether it improves or makes the performance worse?
Then how about making it a per-query option?
Query hints, or session variables (hard to control if there are multiple joins in a single query)

Or may_exist not good enough?

[fuyufjh]

Interesting 🤔 cc. @hzxa21 Any ideas about the reason?

[hzxa21]

TL;DR, I think in the perf benchmark, the locality between the two stream is worse than expected so refilling cache on write in the join executor cause eviction on the old entries that are still needed, leading to higher operator cache miss. cc @KeXiangWang This is different from what we have seen before. Any thoughts?

Findings after looking at grafana (up: 02/23 no may_exist and join cache refill on write, down: 02/24):

1. Aggregation Cached Keys is way smaller in 02/24
   
   

2. Join Cached Entries is also way smaller in 02/24
   
   

3. Join cache miss rate is higher. Cache misses when insert entries in the left table is high and the miss keys are most likely existing keys (may_exists true rate is high) in 02/24
   
   

4. Read duration of the join left and right table are way higher in 02/24
   
   

[KeXiangWang]

I agree with Patrick. Nexmark Q7 is join price of bids and the maxprice of bids within a interval. With refilling, all the bid will be cache in the join cache. But you can image, maxprice stream's event is sparse and only read several keys of price. So, I believe the throughput decrease because of two reason:

1. Unnecessary call of may_exist when inserting a price event intruduce overhead.
2. price event occupy too much cache, causing cache miss and thus latences.

Like @lmatz mentioned, we may probably make decision depends on task and data patterns😕.

[github-actions]

This issue has been open for 60 days with no activity. Could you please update the status? Feel free to continue discussion or close as not planned.

[lmatz]

Later, we found that the 8 kafka-partition nexmark is generated by 16 threads, which can mess up the locality.
Now I suppose this factor may affect the effectiveness of refilling.

I will try to re-enable it later and make it a configurable choice.

[github-actions]

This issue has been open for 60 days with no activity.

If you think it is still relevant today, and needs to be done in the near future, you can comment to update the status, or just manually remove the no-issue-activity label.

You can also confidently close this issue as not planned to keep our backlog clean.
Don't worry if you think the issue is still valuable to continue in the future.
It's searchable and can be reopened when it's time. 😄