github-actions[bot] commented on code in PR #67639:
URL: https://github.com/apache/doris/pull/67639#discussion_r4375388511
##########
cloud/src/meta-service/txn_lazy_committer.cpp:
##########
@@ -45,6 +47,26 @@ using namespace std::chrono;
namespace doris::cloud {
+bool try_append_txn_owner_candidate(TxnInfoPB* txn_info, int64_t tablet_id) {
+ txn_info->add_last_active_candidate_tablet_ids(tablet_id);
+ if (txn_info->ByteSizeLong() <= kTxnInfoOwnerMetadataSoftLimit) {
Review Comment:
[P2] Keep owner-notification construction linear
Each append calls `ByteSizeLong()`, which recomputes the sizes of all packed
int64 entries already present ([protobuf 21.11's sizing
loop](https://github.com/protocolbuffers/protobuf/blob/v21.11/src/google/protobuf/wire_format_lite.cc#L785-L791)).
`commit_txn_eventually` invokes this once per candidate tablet while its KV
transaction is open; the result helper below repeats the pattern for two arrays
during immediate/subtransaction commits and lazy VISIBLE finalization. A load
with 28,500 three-byte tablet IDs fits the 90KB candidate cap (85,500 payload
bytes plus a small TxnInfo envelope), but constructing that list performs
406,139,250 accumulated integer-sizing iterations. The byte cap therefore still
permits quadratic CPU work in mandatory commit processing. Compute the initial
envelope size once and account incrementally for new varints and packed-field
length prefixes, or construct/truncate the lists in a linear pass while
preserving the exact serialized-size bound.
##########
be/src/agent/task_worker_pool.cpp:
##########
@@ -2535,6 +2536,33 @@ void
make_cloud_committed_rs_visible_callback(CloudStorageEngine& engine,
int64_t version_update_time_ms =
make_visible_req.__isset.version_update_time_ms
?
make_visible_req.version_update_time_ms
: 0;
+ std::string load_cluster_id =
+ make_visible_req.__isset.load_cluster_id ?
make_visible_req.load_cluster_id : "";
+ if (config::enable_compaction_rw_separation && !load_cluster_id.empty() &&
+ make_visible_req.__isset.last_active_tablet_ids) {
+ const bool has_aligned_epochs =
make_visible_req.__isset.last_active_epochs &&
+
make_visible_req.last_active_epochs.size() ==
+
make_visible_req.last_active_tablet_ids.size();
+ for (size_t i = 0; i < make_visible_req.last_active_tablet_ids.size();
++i) {
+ auto tablet_result = tablet_mgr.get_tablet(
Review Comment:
[P2] Use a quiet cache lookup for broadcast owner updates
FE now sends the complete owner-tablet list to every alive BE in the load
group, so many lookups here are expected cache misses. However,
`CloudTabletMgr::get_tablet(... force_use_only_cached=true)` unconditionally
emits an INFO log and constructs an InternalError for each miss
(`cloud_tablet_mgr.cpp:203-212`), before this loop ignores it. With 1,024
tablets distributed across 16 BEs caching only their own 64 writer tablets, one
successful commit produces 15,360 such log records; repeated loads multiply the
log I/O and worker CPU across the group. The existing `get_tablet_if_cached`
returns a shared pointer without RPC or logging. Use that quiet probe and skip
missing tablets here and in `consume_commit_owner_updates`, preserving the
intended full-group owner refresh without logging every uncached tablet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
