Skip to content

fix transaction too large error when use TiDB as storage#5754

Closed
moonsphere wants to merge 5 commits into
apache:masterfrom
moonsphere:dev-fix-tdb
Closed

fix transaction too large error when use TiDB as storage#5754
moonsphere wants to merge 5 commits into
apache:masterfrom
moonsphere:dev-fix-tdb

Conversation

@moonsphere
Copy link
Copy Markdown

@moonsphere moonsphere commented Oct 29, 2020

Closes #5749

TiDB can not delete many rows in one transaction.

so maybe we can use @xxxlxy2008 's genius idea --- split one delete stmt into multiple delete stmt with limit and loop until all wanted data has be deleted

to detect affect-row, users need ensure "useAffectedRows=true" must be configured in jdbc url.

to don't affect MySQL logic, we can extend a new TiDBStorageProvider, TiDB user can use

storage:
  tidb:

to replace

storage:
  mysql:

to use this patch.

remain question: delete ... limit N will became slower due to TiDB need scan delete tombstone again again, but maybe it can be improve in future, this PR just handle "transaction too large error"

Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Seems you miss the update of default application.yaml in the bootstrap module, configuration vocabulary doc after you changed the YAML, and storage setup doc.

import java.sql.Connection;
import java.sql.SQLException;

public class TiDBHistoryDeleteDAO extends H2HistoryDeleteDAO {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add comments about why you need this.

* caution: need add "useAffectedRows=true" to jdbc url.
*/
@Slf4j
public class TiDBStorageProvider extends MySQLStorageProvider {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you have a new provider, we need a new maven module for tidb storage, and make it compiled as a separate jar.

@wu-sheng wu-sheng added backend OAP backend related. plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. feature New feature labels Oct 29, 2020
@wu-sheng wu-sheng requested a review from kezhenxu94 October 29, 2020 13:26
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 29, 2020

Codecov Report

Merging #5754 into master will decrease coverage by 51.47%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #5754       +/-   ##
============================================
- Coverage     51.47%   0.00%   -51.48%     
============================================
  Files          1644     161     -1483     
  Lines         35215    3914    -31301     
  Branches       3841     459     -3382     
============================================
- Hits          18127       0    -18127     
+ Misses        16186    3914    -12272     
+ Partials        902       0      -902     
Impacted Files Coverage Δ Complexity Δ
...king/apm/util/RunnableWithExceptionProtection.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-2.00%)
...ywalking/apm/agent/core/logging/core/LogLevel.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-1.00%)
...ywalking/apm/agent/core/plugin/EnhanceContext.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-5.00%)
...ywalking/apm/agent/core/plugin/PluginSelector.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-2.00%)
...ywalking/apm/agent/core/profile/ProfileStatus.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-1.00%)
...walking/apm/agent/core/context/SW8CarrierItem.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-2.00%)
...walking/apm/agent/core/jvm/cpu/SunCpuAccessor.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-2.00%)
...walking/apm/agent/core/logging/core/LogOutput.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-1.00%)
...walking/apm/agent/core/plugin/match/NameMatch.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-3.00%)
...alking/apm/agent/core/context/CarrierItemHead.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-1.00%)
... and 1620 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 268ace5...8b13719. Read the comment docs.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Nov 1, 2020

@moonsphere I have a direct talk with PingCap's friends, right now, as a minor change, this deletion way seems the best choice.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Nov 7, 2020

@moonsphere Are you still working on this?

@JaredTan95 JaredTan95 mentioned this pull request Nov 14, 2020
3 tasks
@wu-sheng
Copy link
Copy Markdown
Member

As no update, this is going to be replaced by #5844

@wu-sheng wu-sheng closed this Nov 15, 2020
@wu-sheng wu-sheng added this to the 8.3.0 milestone Nov 15, 2020
@wu-sheng wu-sheng added the no update The owner doesn't provide further feedback. label Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend OAP backend related. feature New feature no update The owner doesn't provide further feedback. plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transaction is too large when use TiDB as storage

3 participants