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

[improve](txn insert) make sub transactions visible #41362

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mymeiyi
Copy link
Contributor

@mymeiyi mymeiyi commented Sep 26, 2024

this pr does not support mow table.

mow support is in #42089 or #43011

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Sep 26, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.31% (9635/25827)
Line Coverage: 28.70% (79715/277800)
Region Coverage: 28.12% (41217/146573)
Branch Coverage: 24.75% (20989/84804)
Coverage Report: http://coverage.selectdb-in.cc/coverage/468f4a1ee092489aeb84d542f4c98fb7d4593b82_468f4a1ee092489aeb84d542f4c98fb7d4593b82/report/index.html

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Sep 27, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.29% (9627/25815)
Line Coverage: 28.68% (79665/277737)
Region Coverage: 28.12% (41191/146504)
Branch Coverage: 24.74% (20973/84788)
Coverage Report: http://coverage.selectdb-in.cc/coverage/7646c207b62a29bf04a45757bfe5eef2e8e9489b_7646c207b62a29bf04a45757bfe5eef2e8e9489b/report/index.html

@@ -1743,6 +1745,9 @@ public void setEnableLeftZigZag(boolean enableLeftZigZag) {
@VariableMgr.VarAttr(name = GROUP_COMMIT, needForward = true)
public String groupCommit = "off_mode";

@VariableMgr.VarAttr(name = ENABLE_QUERY_IN_TRANSACTION_LOAD, needForward = true)
public boolean enableQueryInTransactionLoad = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

default should be true.

Copy link
Contributor

Choose a reason for hiding this comment

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

enable_transaction_write_visibility is a better name.

ConnectContext.get().getTxnEntry().getTabletSubTxnIds(olapTable.getId(), tablet));
LOG.info("table={}, partition={}, tablet={}, sub txn ids={}", olapTable.getId(), partition.getId(),
tablet.getId(), paloRange.getSubTxnIds());
}

Copy link
Contributor

@dataroaring dataroaring Sep 27, 2024

Choose a reason for hiding this comment

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

We should assure that select in txn is forwarded to master too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the observer also knows the sub txn info

DCHECK(rowset != nullptr) << " rowset is nullptr for sub_txn_id=" << sub_txn_ids[i]
<< ", partition_id=" << partition_id()
<< ", tablet=" << tablet_id();
int64_t tmp_version = version + i + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

version has already been added in prallel_scanner_builder _build_scanners_by_rowid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_load is called firstly, now version is not changed

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Sep 29, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.29% (9627/25820)
Line Coverage: 28.67% (79675/277888)
Region Coverage: 28.11% (41208/146593)
Branch Coverage: 24.73% (20982/84848)
Coverage Report: http://coverage.selectdb-in.cc/coverage/3b07d43331c05277cc24a34fb911921877e73809_3b07d43331c05277cc24a34fb911921877e73809/report/index.html

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Sep 29, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.28% (9625/25820)
Line Coverage: 28.68% (79689/277888)
Region Coverage: 28.11% (41202/146593)
Branch Coverage: 24.72% (20976/84848)
Coverage Report: http://coverage.selectdb-in.cc/coverage/78739c1820daadfd7345065e11f836ab27e2a2a7_78739c1820daadfd7345065e11f836ab27e2a2a7/report/index.html

@mymeiyi mymeiyi force-pushed the sub-txn-visible branch 2 times, most recently from b730606 to db8cdc0 Compare September 30, 2024 08:19
@mymeiyi
Copy link
Contributor Author

mymeiyi commented Sep 30, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.31% (9631/25811)
Line Coverage: 28.68% (79853/278428)
Region Coverage: 28.13% (41301/146824)
Branch Coverage: 24.75% (21043/85018)
Coverage Report: http://coverage.selectdb-in.cc/coverage/db8cdc062e4c6790ffc93255722c460ee3a49c5a_db8cdc062e4c6790ffc93255722c460ee3a49c5a/report/index.html

@mymeiyi mymeiyi force-pushed the sub-txn-visible branch 2 times, most recently from 393fb19 to e4fe448 Compare October 12, 2024 03:33
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -128,6 +128,42 @@ BaseTablet::~BaseTablet() {
g_total_tablet_num << -1;
}

Status BaseTablet::capture_sub_txn_rs_readers(int64_t version,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'capture_sub_txn_rs_readers' can be made const [readability-make-member-function-const]

be/src/olap/base_tablet.cpp:132:

-                                               std::vector<RowSetSplits>* rs_splits) {
+                                               std::vector<RowSetSplits>* rs_splits) const {

// unique keys with merge on write, no need to merge sort keys in rowset
need_ordered_result = false;
if (read_params.query_mow_in_mor) {
need_ordered_result = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: redundant boolean literal in conditional assignment [readability-simplify-boolean-expr]

                need_ordered_result = true;
                                      ^

@@ -71,7 +71,11 @@ void VCollectIterator::init(TabletReader* reader, bool ori_data_overlapping, boo
(_reader->_direct_mode || _reader->_tablet->keys_type() == KeysType::DUP_KEYS ||
(_reader->_tablet->keys_type() == KeysType::UNIQUE_KEYS &&
_reader->_tablet->enable_unique_key_merge_on_write()))) {
_merge = false;
if (_reader->_reader_context.query_mow_in_mor) {
_merge = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: redundant boolean literal in conditional assignment [readability-simplify-boolean-expr]

be/src/vec/olap/vcollect_iterator.cpp:73:

-         if (_reader->_reader_context.query_mow_in_mor) {
-             _merge = true;
-         } else {
-             _merge = false;
-         }
+         _merge = static_cast<bool>(_reader->_reader_context.query_mow_in_mor);

@@ -1410,6 +1410,108 @@ static bool try_fetch_and_parse_schema(Transaction* txn, RowsetMetaCloudPB& rows
return true;
}

void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController* controller,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'get_tmp_rowset' exceeds recommended size/complexity thresholds [readability-function-size]

void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController* controller,
                      ^
Additional context

cloud/src/meta-service/meta_service.cpp:1412: 97 lines including whitespace and comments (threshold 80)

void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController* controller,
                      ^

return;
}
// set tablet schema
if (rowset_meta->has_tablet_schema()) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: statement should be inside braces [readability-braces-around-statements]

Suggested change
if (rowset_meta->has_tablet_schema()) continue;
if (rowset_meta->has_tablet_schema()) { continue;
}

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Oct 12, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.39% (9659/25834)
Line Coverage: 28.64% (80131/279747)
Region Coverage: 28.07% (41435/147608)
Branch Coverage: 24.67% (21103/85526)
Coverage Report: http://coverage.selectdb-in.cc/coverage/e4fe4482ba7182ab619af8fb1fe75db39aac9762_e4fe4482ba7182ab619af8fb1fe75db39aac9762/report/index.html

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Oct 12, 2024

run buildall

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Oct 31, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.92% (9850/25978)
Line Coverage: 29.17% (82053/281274)
Region Coverage: 28.44% (42347/148918)
Branch Coverage: 25.02% (21517/86014)
Coverage Report: http://coverage.selectdb-in.cc/coverage/26b0fdc8ea349ff089d07e1bb2b715aa59b661c8_26b0fdc8ea349ff089d07e1bb2b715aa59b661c8/report/index.html

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Oct 31, 2024

run p0

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Nov 4, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.80% (9818/25972)
Line Coverage: 28.95% (81558/281685)
Region Coverage: 28.23% (42126/149214)
Branch Coverage: 24.81% (21373/86142)
Coverage Report: http://coverage.selectdb-in.cc/coverage/ee8e7345872a92faf689d92b91b4e07122f36afc_ee8e7345872a92faf689d92b91b4e07122f36afc/report/index.html

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Nov 8, 2024

run buildall

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -129,6 +129,42 @@ BaseTablet::~BaseTablet() {
g_total_tablet_num << -1;
}

Status BaseTablet::capture_sub_txn_rs_readers(int64_t version,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'capture_sub_txn_rs_readers' can be made const [readability-make-member-function-const]

be/src/olap/base_tablet.cpp:133:

-                                               std::vector<RowSetSplits>* rs_splits) {
+                                               std::vector<RowSetSplits>* rs_splits) const {

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.89% (9864/26031)
Line Coverage: 29.06% (82069/282456)
Region Coverage: 28.27% (42266/149485)
Branch Coverage: 24.85% (21436/86278)
Coverage Report: http://coverage.selectdb-in.cc/coverage/21597e29e7b140083f2e25ca1a8834c3a9ba5716_21597e29e7b140083f2e25ca1a8834c3a9ba5716/report/index.html

@dataroaring
Copy link
Contributor

run buildall

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Nov 12, 2024

run buildall

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Nov 12, 2024

run buildall

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -1418,6 +1418,108 @@ static bool try_fetch_and_parse_schema(Transaction* txn, RowsetMetaCloudPB& rows
return true;
}

void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController* controller,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'get_tmp_rowset' exceeds recommended size/complexity thresholds [readability-function-size]

void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController* controller,
                      ^
Additional context

cloud/src/meta-service/meta_service.cpp:1420: 97 lines including whitespace and comments (threshold 80)

void MetaServiceImpl::get_tmp_rowset(::google::protobuf::RpcController* controller,
                      ^

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Nov 13, 2024

run buildall

Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 19, 2024
Copy link
Contributor

PR approved by at least one committer and no changes requested.

Copy link
Contributor

PR approved by anyone and no changes requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants