Skip to content

Fix bug for ingesting data to a "pk is handle" table (#2125)#2129

Merged
JaySon-Huang merged 2 commits intopingcap:release-5.1from
ti-srebot:release-5.1-46b829485358
Jun 11, 2021
Merged

Fix bug for ingesting data to a "pk is handle" table (#2125)#2129
JaySon-Huang merged 2 commits intopingcap:release-5.1from
ti-srebot:release-5.1-46b829485358

Conversation

@ti-srebot
Copy link
Collaborator

@ti-srebot ti-srebot commented Jun 10, 2021 •
edited
Loading

cherry-pick #2125 to release-5.1
You can switch your code base to this Pull Request by using git-extras:

# In tics repo:
git pr https://github.com/pingcap/tics/pull/2129

After apply modifications, you can push your change to this PR via:

git push git@github.com:ti-srebot/tics.git pr/2129:release-5.1-46b829485358

What problem does this PR solve?

Issue Number: close #2118

Problem Summary:

DeltaTree add a "_tidb_rowid" column to the table even if "pk is handle" is true. We use the FunctionToInt64 to copy data from the primary key column to the handle column under this situation.

https://github.com/pingcap/tics/blob/1078d0b8199a1cfe54d73623ea8da636505f419b/dbms/src/Storages/DeltaMerge/DeltaMergeStore.cpp#L403-L410

However, if the types are identical, FunctionToInt64 only does a shallow copy and makes the primary key column and _tidb_rowid share the same column ptr when "pk is handle".

https://github.com/pingcap/tics/blob/1078d0b8199a1cfe54d73623ea8da636505f419b/dbms/src/Functions/FunctionsConversion.h#L832-L841

After that, we need to reorganize the boundary of blocks, this makes us append some rows into the pk column and handle column. Unfortunately, they share the same column pointer, which makes the columns inside one block don't align and make trouble for later processing.
https://github.com/pingcap/tics/blob/1078d0b8199a1cfe54d73623ea8da636505f419b/dbms/src/Storages/DeltaMerge/PKSquashingBlockInputStream.h#L75-L99

What is changed and how it works?

Use deep copy instead of shallow copy when the types are identical in DeltaMergeStore::addExtraColumnIfNeed.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch:

Check List

Tests

  • Unit test

Side effects

Release note

  • No release note

@JaySon-Huang @ti-srebot
cherry pick #2125 to release-5.1
d7b6ca1 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Jun 10, 2021
Merged
@ti-srebot ti-srebot added CHERRY-PICK cherry pick priority/release-blocker This issue blocks a release. Please solve it ASAP. status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug. labels Jun 10, 2021
@ti-srebot ti-srebot requested a review from flowbehappy June 10, 2021 05:36
@ti-srebot ti-srebot added this to the v5.1.0 milestone Jun 10, 2021
@JaySon-Huang

This comment has been minimized.

Sign in to view
@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021

/cc @flowbehappy @zhouqiang-cl

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021

/rebuild

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021

/run-all-tests

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021 •
edited
Loading

The docker image hub.pingcap.net/qa/tidb:release-5.1-failpoint is missing

[2021-06-10T06:12:59.963Z] Pulling tidb0 (hub.pingcap.net/qa/tidb:release-5.1-failpoint)...
[2021-06-10T06:12:59.963Z] manifest for hub.pingcap.net/qa/tidb:release-5.1-failpoint not found

some other stages are fail because of

[2021-06-10T06:23:56.799Z] ERROR: Queue task was cancelled

Link: https://ci.pingcap.net/blue/organizations/jenkins/tics_ghpr_test/detail/tics_ghpr_test/411/pipeline/176/

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021

/run-all-tests

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021 •
edited
Loading 

[2021-06-10T07:09:26.807Z] [Pipeline] // parallel
[2021-06-10T07:09:26.811Z] [Pipeline] }
[2021-06-10T07:09:26.813Z] ERROR: Queue task was cancelled
[2021-06-10T07:09:26.821Z] [Pipeline] // catchError
[2021-06-10T07:09:26.827Z] [Pipeline] stage
[2021-06-10T07:09:26.828Z] [Pipeline] { (Summary)
[2021-06-10T07:09:26.838Z] [Pipeline] echo
[2021-06-10T07:09:26.839Z] Result: `FAILURE`

https://ci.pingcap.net/blue/organizations/jenkins/tics_ghpr_test/detail/tics_ghpr_test/414/pipeline/67

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021

/run-all-tests

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021

/run-all-tests

1 similar comment
@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021

/run-all-tests

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 10, 2021

Waiting on bug triage

flowbehappy
flowbehappy approved these changes Jun 10, 2021
View reviewed changes
Copy link
Contributor

@flowbehappy flowbehappy left a comment

Choose a reason for hiding this comment

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

LGTM

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 11, 2021

Bug triage done, merging this fix.

@JaySon-Huang JaySon-Huang merged commit fb9df56 into pingcap:release-5.1 Jun 11, 2021
@JaySon-Huang JaySon-Huang deleted the release-5.1-46b829485358 branch June 11, 2021 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flowbehappy flowbehappy flowbehappy approved these changes

@zhouqiang-cl zhouqiang-cl Awaiting requested review from zhouqiang-cl

Assignees

@JaySon-Huang JaySon-Huang

Labels

CHERRY-PICK cherry pick priority/release-blocker This issue blocks a release. Please solve it ASAP. status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.

Projects

None yet

Milestone

v5.1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@ti-srebot @JaySon-Huang @flowbehappy @solotzg