Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stats: reduce conflict for updating column size #7124

Merged
merged 6 commits into from
Jul 24, 2018

Conversation

alivxxx
Copy link
Contributor

@alivxxx alivxxx commented Jul 23, 2018

What have you changed? (mandatory)

By using insert ... on duplicate, we can reduce the possibility of conflict because of less execution time.
Fix #7079.
PTAL @coocood @zz-jason @winoros

What is the type of the changes? (mandatory)

  • Improvement (non-breaking change which is an improvement to an existing feature)

How has this PR been tested? (mandatory)

Existing unit test.

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

No.

Does this PR affect tidb-ansible update? (mandatory)

No.

Does this PR need to be added to the release notes? (mandatory)

No.

if err != nil {
return
}
values = append(values, fmt.Sprintf("(%d, 0, %d, 0, %d)", id, key, val))
Copy link
Member

Choose a reason for hiding this comment

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

The val is delta, we need to insert the new value.


sql := fmt.Sprintf("select * from mysql.stats_histograms where table_id = %d and is_index = 0 for update", id)
Copy link
Member

Choose a reason for hiding this comment

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

We should select needed columns instead of * for compatibility.

@@ -344,17 +345,16 @@ func (h *Handle) dumpTableStatColSizeToKV(id int64, delta variable.TableDelta) (
err = finishTransaction(ctx, exec, err)
}()
version := h.mu.ctx.Txn().StartTS()

values := make([]string, 0, len(delta.ColSize))
for key, val := range delta.ColSize {
Copy link
Member

Choose a reason for hiding this comment

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

rename to histID, deltaColSize would be more clear.

@coocood
Copy link
Member

coocood commented Jul 23, 2018

/run-all-tests

@lysu
Copy link
Contributor

lysu commented Jul 23, 2018

/run-integration-ddl-test

@alivxxx alivxxx added status/all tests passed component/statistics type/enhancement The issue or PR belongs to an enhancement. labels Jul 23, 2018
@jackysp
Copy link
Member

jackysp commented Jul 23, 2018

How to know that it is effective?

@coocood
Copy link
Member

coocood commented Jul 23, 2018

@lamxTyler we need to do a manual test and verify that the conflict is reduced.

@alivxxx
Copy link
Contributor Author

alivxxx commented Jul 23, 2018

Two concurrent writers, update 10 columns for 200 times, a 10-microsecond ticker for each update, this PR has total 23 conflicts, while the master has 135 conflicts. @jackysp @coocood

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@alivxxx alivxxx merged commit 46e46c1 into pingcap:master Jul 24, 2018
@alivxxx alivxxx deleted the colsize branch July 24, 2018 03:49
alivxxx added a commit to alivxxx/tidb that referenced this pull request Jul 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/statistics type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants