Skip to content

Conversation

@gavinchou
Copy link
Contributor

@gavinchou gavinchou commented Jan 8, 2025

What problem does this PR solve?

This PR read recycle_rowset_key when commit_rowset, and it also fix some cloud UTs: loopaddr, recycler JVM memory, resource drop FE.
Related PR #45255 #46798

We should read the recycle_rowset_key before removing to make KV txn serializable.
Test case (hard to make it a regression)

mysql -h127.0.0.1 -P8952 -uroot -Ddb1 -e 'drop table if exists t1'
mysql -h127.0.0.1 -P8952 -uroot -Ddb1 -e 'CREATE TABLE t1 (id int, name text, score text) ENGINE=OLAP DUPLICATE KEY(id) DISTRIBUTED BY HASH(id) BUCKETS 1'

curl -XPOST "localhost:7211/api/update_config?disable_auto_compaction=true"
curl "localhost:6000/MetaService/http/v1/injection_point?token=greedisgood9999&op=disable"
curl "localhost:6000/MetaService/http/v1/injection_point?token=greedisgood9999&op=clear"

for i in $(seq 1 1 100); do
  mysql -h127.0.0.1 -P8952 -uroot -Ddb1 -e 'insert into t1 values(1,1,1)'
done

curl "localhost:6000/MetaService/http/v1/injection_point?token=greedisgood9999&op=apply_suite&name=sleep_before_execute_commit_rowset"
curl "localhost:6000/MetaService/http/v1/injection_point?token=greedisgood9999&op=enable"
curl -XPOST "localhost:7211/api/update_config?disable_auto_compaction=false"

sleep 60

instance_id=gavin_debug_instance_doris
tablet_id=$(mysql -h127.0.0.1 -P8952 -uroot -Ddb1 -e 'show tablets from t1;' | awk '{if (++n>1) print $1}')

curl "localhost:7211/api/compaction/show?tablet_id=${tablet_id}"

txn_id=$(curl -s "localhost:6000/MetaService/http/get_value?token=greedisgood9999&unicode&key_type=MetaRowsetKey&instance_id=${instance_id}&tablet_id=${tablet_id}&version=101" | jq ".txn_id" | tr -d '"')

curl -s "localhost:6000/MetaService/http/get_value?token=greedisgood9999&unicode&key_type=MetaRowsetTmpKey&instance_id=${instance_id}&tablet_id=${tablet_id}&txn_id=${txn_id}"

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen
Copy link
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@gavinchou
Copy link
Contributor Author

run buildall

w41ter
w41ter previously approved these changes Jan 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2025

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Jan 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2025

PR approved by anyone and no changes requested.

…it rowset

Test case (hard to make it a regression)
```
mysql -h127.0.0.1 -P8952 -uroot -Ddb1 -e 'drop table if exists t1'
mysql -h127.0.0.1 -P8952 -uroot -Ddb1 -e 'CREATE TABLE t1 (id int, name text, score text) ENGINE=OLAP DUPLICATE KEY(id) DISTRIBUTED BY HASH(id) BUCKETS 1'

curl -XPOST "localhost:7211/api/update_config?disable_auto_compaction=true"
curl "localhost:6000/MetaService/http/v1/injection_point?token=greedisgood9999&op=disable"
curl "localhost:6000/MetaService/http/v1/injection_point?token=greedisgood9999&op=clear"

for i in $(seq 1 1 100); do
  mysql -h127.0.0.1 -P8952 -uroot -Ddb1 -e 'insert into t1 values(1,1,1)'
done

curl "localhost:6000/MetaService/http/v1/injection_point?token=greedisgood9999&op=apply_suite&name=sleep_before_execute_commit_rowset"
curl "localhost:6000/MetaService/http/v1/injection_point?token=greedisgood9999&op=enable"
curl -XPOST "localhost:7211/api/update_config?disable_auto_compaction=false"

sleep 60

instance_id=gavin_debug_instance_doris
tablet_id=$(mysql -h127.0.0.1 -P8952 -uroot -Ddb1 -e 'show tablets from t1;' | awk '{if (++n>1) print $1}')

curl "localhost:7211/api/compaction/show?tablet_id=${tablet_id}"

txn_id=$(curl -s "localhost:6000/MetaService/http/get_value?token=greedisgood9999&unicode&key_type=MetaRowsetKey&instance_id=${instance_id}&tablet_id=${tablet_id}&version=101" | jq ".txn_id" | tr -d '"')

curl -s "localhost:6000/MetaService/http/get_value?token=greedisgood9999&unicode&key_type=MetaRowsetTmpKey&instance_id=${instance_id}&tablet_id=${tablet_id}&txn_id=${txn_id}"
```
@gavinchou gavinchou force-pushed the gavin-fix-commit-rowset branch from 27c85a1 to 7dda53f Compare January 18, 2025 17:37
@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Jan 18, 2025
@gavinchou
Copy link
Contributor Author

run buildall

Copy link
Contributor

@deardeng deardeng left a comment

Choose a reason for hiding this comment

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

LGTM

@gavinchou
Copy link
Contributor Author

run buildall

@gavinchou
Copy link
Contributor Author

run buildall

@gavinchou gavinchou removed the p0_c label Jan 26, 2025
@gavinchou
Copy link
Contributor Author

run buildall

@gavinchou gavinchou marked this pull request as draft May 27, 2025 05:15
@gavinchou
Copy link
Contributor Author

close due to #51048 #51129

@gavinchou gavinchou closed this Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants