Skip to content

[fix](cloud-mow) get delete bitmap update lock should create a new Transaction when encounter TXN_TOO_OLD - #44691

Closed
hust-hhb wants to merge 2 commits into
apache:masterfrom
hust-hhb:fix-get-lock
Closed

[fix](cloud-mow) get delete bitmap update lock should create a new Transaction when encounter TXN_TOO_OLD#44691
hust-hhb wants to merge 2 commits into
apache:masterfrom
hust-hhb:fix-get-lock

Conversation

@hust-hhb

@hust-hhbhust-hhb commented Nov 27, 2024

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Related PR: #37670

Problem Summary:
When the number of tablet is big, get delete bitmap update lock may cost over 5 seconds, which will lead to transaction timeout, we should create a new transaction to get the remaining tablet stats.

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

@doris-robot

doris-robot commented Nov 27, 2024

Copy link
Copy Markdown

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?

@hust-hhb

hust-hhb commented Nov 27, 2024

Copy link
Copy Markdown
ContributorAuthor

run buildall

zhannngchen
zhannngchen previously approved these changes Dec 3, 2024
View reviewed changes

@zhannngchenzhannngchen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Dec 3, 2024
@github-actions

github-actionsBot commented Dec 3, 2024

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actionsBot commented Dec 3, 2024

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

gavinchou
gavinchou requested changes Dec 3, 2024
View reviewed changes
Comment threadcloud/src/meta-service/meta_service_tablet_stats.cpp Outdated

void internal_get_tablet_stats(MetaServiceCode& code, std::string& msg, Transaction* txn,
void internal_get_tablet_stats(MetaServiceCode& code, std::string& msg,
std::unique_ptr<Transaction>& txn, std::shared_ptr<TxnKv>& txn_kv,

@gavinchougavinchouDec 3, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a behavior change, semantic changed, txn atomic is not garanteed if we create a new txn.
Fix where this internal_get_tablet_stats() is called if we met TXN_TOO_OLD

@hust-hhbhust-hhbDec 4, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@gavinchou already change the code, take a look please

@github-actionsgithub-actionsBot removed the approved Indicates a PR has been approved by one committer. label Dec 3, 2024
@hust-hhb

hust-hhb commented Dec 4, 2024

Copy link
Copy Markdown
ContributorAuthor

run buildall

@hust-hhb

hust-hhb commented Dec 4, 2024

Copy link
Copy Markdown
ContributorAuthor

run buildall

gavinchou
gavinchou reviewed Dec 9, 2024
View reviewed changes
Comment threadcloud/src/meta-service/meta_service.cpp
code = MetaServiceCode::OK;
txn = nullptr;
retry++;
err = txn_kv_->create_txn(&txn);

@gavinchougavinchouDec 9, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

overwrite the txn cannot modify the "delete_bitmap_upldate_lock"

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gavinchougavinchougavinchou requested changes
@zhannngchenzhannngchenzhannngchen left review comments

Assignees

No one assigned

Labels

reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hust-hhb@doris-robot@gavinchou@zhannngchen