Skip to content

Support txn management in db level isolation and use ArrayDeque to improve txn task performance - #3369

Merged
morningman merged 20 commits into
apache:masterfrom
caiconghui:db_txn
May 11, 2020
Merged

Support txn management in db level isolation and use ArrayDeque to improve txn task performance#3369
morningman merged 20 commits into
apache:masterfrom
caiconghui:db_txn

Conversation

@caiconghui

@caiconghuicaiconghui commented Apr 21, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

This PR is the first step to make doris stream load more robust with higher concurrent performance(#3368)，the main work is to support txn management in db level isolation，and use ArrayDeque to stored final status txns

@chaoylichaoyli added this to the 0.13.0 milestone Apr 22, 2020
@chaoylichaoyli linked an issue Apr 22, 2020 that may be closed by this pull request
[Proposal] Making doris stream load more robust with higher concurrent performance #3368
Closed
morningman
morningman reviewed Apr 24, 2020
View reviewed changes
Comment threadfe/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java
Comment threadfe/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java
Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
morningman
morningman requested changes May 5, 2020
View reviewed changes
Comment threadfe/src/main/java/org/apache/doris/service/FrontendServiceImpl.java Outdated
}

Catalog.getCurrentGlobalTransactionMgr().abortTransaction(request.getTxnId(),
long dbId = Catalog.getInstance().getDb(request.getDb()).getId();

@morningmanmorningmanMay 5, 2020

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.

getDb() may return null if database does not exist yet.

Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
public DatabaseTransactionMgr getDatabaseTransactioMgr(long dbId) {
DatabaseTransactionMgr dbTransactionMgr = dbIdToDatabaseTransactionMgrs.get(dbId);
if (dbTransactionMgr == null) {
throw new NullPointerException("databaseTransactionMgr[" + dbId + "] does not exist");

@morningmanmorningmanMay 5, 2020

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.

Not using NullPointerException, you can use TransactionException

Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
}

public void addDatabaseTransactionMgr(Long dbId, EditLog editLog) {
dbIdToDatabaseTransactionMgrs.put(dbId, new DatabaseTransactionMgr(dbId, editLog));

@morningmanmorningmanMay 5, 2020

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.

Is it more safe to use putIfAbsent() method?

Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
}

checkRunningTxnExceedLimit(dbId, sourceType);
checkRunningTxnExceedLimit(dbTransactionMgr, sourceType);

@morningmanmorningmanMay 5, 2020

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.

I think checkRunningTxnExceedLimit() this method can be moved into the DbTransactionMgr

Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
@morningman

morningman commented May 5, 2020

Copy link
Copy Markdown
Contributor

@caiconghui I have some comments left. And could you please resolve the conflicts?

@morningmanmorningman self-assigned this May 5, 2020
@caiconghui
caiconghuiforce-pushed the db_txn branch 2 times, most recently from c6fb244 to 4f38eb0CompareMay 6, 2020 11:03
@morningmanmorningman added area/load Issues or PRs related to all kinds of load area/catalog Issues or PRs related to catalog management labels May 7, 2020
morningman
morningman requested changes May 7, 2020
View reviewed changes
Comment threadfe/src/main/java/org/apache/doris/catalog/Catalog.java Outdated
Comment threadfe/src/main/java/org/apache/doris/catalog/Catalog.java Outdated
Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
public DatabaseTransactionMgr getDatabaseTransactioMgr(long dbId) throws AnalysisException {
DatabaseTransactionMgr dbTransactionMgr = dbIdToDatabaseTransactionMgrs.get(dbId);
if (dbTransactionMgr == null) {
throw new AnalysisException("databaseTransactionMgr[" + dbId + "] does not exist");

@morningmanmorningmanMay 7, 2020

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.

AnalysisException is not suitable here.
But it can be modified next time

Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
}
}
return txnIds;
return txnInfos.size() > limit ? new ArrayList<>(txnInfos.subList(0, limit)) : txnInfos;

@morningmanmorningmanMay 7, 2020

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.

Suggested change
returntxnInfos.size() > limit ? newArrayList<>(txnInfos.subList(0, limit)) : txnInfos;
returntxnInfos.size() > limit ? txnInfos.subList(0, limit) : txnInfos;

@caiconghuicaiconghuiMay 7, 2020

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.

@morningman subList is a view of origin list, so is it a good way to directly return sublist?

@morningmanmorningmanMay 10, 2020

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.

I think it's OK, cause no one will modify that list.

Comment threadfe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java Outdated
morningman
morningman approved these changes May 10, 2020
View reviewed changes

@morningmanmorningman 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

@morningmanmorningman added the approved Indicates a PR has been approved by one committer. label May 10, 2020
@morningman
morningman merged commit b648734 into apache:masterMay 11, 2020
This was referenced May 11, 2020
Closed
Closed
Closed
@caiconghui
caiconghui deleted the db_txn branch May 25, 2020 03:07
@EmmyMiao87EmmyMiao87 mentioned this pull request Aug 17, 2020
Closed
acelyc111 pushed a commit to acelyc111/incubator-doris that referenced this pull request Jan 20, 2021
@caiconghui汪聪
[TxxMgr] Support txn management in db level and use ArrayDeque to imp…
f16e252
…rove txn task performance (apache#3369)
This PR is the first step to make Doris stream load more robust with higher concurrent
performance(apache#3368)，the main work is to support txn management in db level isolation
and use ArrayDeque to stored final status txns.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@morningmanmorningmanmorningman approved these changes

Assignees

@morningmanmorningman

@caiconghuicaiconghui

Labels

approvedIndicates a PR has been approved by one committer.area/catalogIssues or PRs related to catalog managementarea/loadIssues or PRs related to all kinds of loadkind/improvement

Projects

None yet

Milestone

0.13.0

Development

Successfully merging this pull request may close these issues.

[Proposal] Making doris stream load more robust with higher concurrent performance

3 participants

@caiconghui@morningman@chaoyli