Skip to content

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