github-actions[bot] commented on code in PR #67401:
URL: https://github.com/apache/doris/pull/67401#discussion_r4250713498
##########
be/src/cloud/cloud_meta_mgr.cpp:
##########
@@ -863,6 +869,10 @@ Status
CloudMetaMgr::sync_tablet_rowsets_unlocked(CloudTablet* tablet,
int64_t now =
duration_cast<seconds>(system_clock::now().time_since_epoch()).count();
tablet->last_sync_time_s = now;
+ if (resp.has_has_alter_job()) {
Review Comment:
[P2] Restore conservative liveness when an old MS omits the field. During a
mixed MS upgrade, the cache-miss get_rowset before job registration can hit a
new MS and set this flag false, while the mandatory post-registration sync hits
an old MS and leaves it unset. This branch preserves false even after
alter_version becomes valid, so the new zombie filter suppresses an active
shadow's compactions. DUP/AGG cache-hit loads do not sync rowsets, and
background sync waits for 1800 seconds of staleness by default; a long
conversion with over 2000 short loads can therefore hit TOO_MANY_VERSION before
repair. Treat an absent response as the documented conservative true fallback,
or refresh liveness on successful registration, and test the false-then-omitted
sequence.
##########
be/src/cloud/cloud_tablet.cpp:
##########
@@ -938,6 +939,62 @@ int64_t CloudTablet::get_cloud_cumu_compaction_score()
const {
return _approximate_cumu_num_deltas.load(std::memory_order_relaxed);
}
+// Keep the conditions consistent with the filter_out/disable/skip logic in
+// CloudStorageEngine::_generate_cloud_compaction_tasks and
+// CloudTabletMgr::get_topn_tablets_to_compact
+std::string CloudTablet::compaction_not_scheduled_reason() {
+ if (is_zombie_tablet()) {
+ return "zombie";
+ }
+ if (_tablet_meta->tablet_schema()->disable_auto_compaction()) {
+ return "disabled";
+ }
+ if (_engine.has_base_compaction(tablet_id()) ||
_engine.has_cumu_compaction(tablet_id()) ||
+ _engine.has_full_compaction(tablet_id()) ||
+ _engine.is_preparing_cumu_compaction(tablet_id())) {
+ return "compaction_inflight";
+ }
+ if (tablet_state() == TABLET_NOTREADY) {
+ if (!config::enable_new_tablet_do_compaction) {
+ return "alter_new_tablet_compaction_disabled";
+ }
+ if (alter_version() == -1) {
+ return "waiting_alter_task";
+ }
+ }
+ auto* cloud_cluster_info =
+
static_cast<CloudClusterInfo*>(ExecEnv::GetInstance()->cluster_info());
+ if (config::enable_standby_passive_compaction &&
cloud_cluster_info->is_in_standby() &&
+ fetch_add_approximate_num_rowsets(0) <
+ config::max_tablet_version_num *
config::standby_compaction_version_ratio) {
+ return "standby_throttled";
+ }
+ if (cloud_cluster_info->should_skip_compaction(this)) {
+ return "handled_by_other_cluster";
+ }
+ int64_t now = std::chrono::duration_cast<std::chrono::milliseconds>(
+ std::chrono::system_clock::now().time_since_epoch())
+ .count();
+ auto cooldown_left = [now](int64_t last_ms) {
+ return last_ms + config::min_compaction_failure_interval_ms - now;
+ };
+ if (int64_t ms = cooldown_left(last_cumu_compaction_failure_time()); ms >
0) {
+ return fmt::format("cumu_failure_cooldown({}ms)", ms);
+ }
+ if (int64_t ms = cooldown_left(last_cumu_no_suitable_version_ms); ms > 0) {
+ return fmt::format("cumu_no_suitable_version({}ms)", ms);
+ }
+ if (int64_t ms = cooldown_left(last_base_compaction_failure_time()); ms >
0) {
Review Comment:
[P2] Keep scheduling reasons specific to the compaction type. With a recent
base-compaction failure but no cumulative cooldown, a RUNNING tablet with
positive cumulative score remains eligible in get_topn_tablets_to_compact, yet
this helper reports base_failure_cooldown. The same mismatch occurs for
compaction_inflight when enable_parallel_cumu_compaction is enabled. These
fields therefore explain a blocker that the real scheduler does not apply.
Evaluate/report eligibility per compaction type (or return a common blocker
only when all applicable types are blocked), using the actual scheduler
conditions.
##########
be/src/cloud/cloud_tablet.cpp:
##########
@@ -938,6 +939,62 @@ int64_t CloudTablet::get_cloud_cumu_compaction_score()
const {
return _approximate_cumu_num_deltas.load(std::memory_order_relaxed);
}
+// Keep the conditions consistent with the filter_out/disable/skip logic in
+// CloudStorageEngine::_generate_cloud_compaction_tasks and
+// CloudTabletMgr::get_topn_tablets_to_compact
+std::string CloudTablet::compaction_not_scheduled_reason() {
+ if (is_zombie_tablet()) {
+ return "zombie";
+ }
+ if (_tablet_meta->tablet_schema()->disable_auto_compaction()) {
Review Comment:
[P2] Report the global automatic-compaction disable switch. When
config::disable_auto_compaction is true, both cloud producer loops skip all
automatic scheduling, but an otherwise eligible tablet returns an empty reason
here because only the per-tablet schema flag is checked. /api/compaction_score
then omits not_scheduled_reason and /api/compaction/show says it is empty, so a
real high backlog has no explanation despite the BE-wide switch stopping it.
Include the global switch in this display predicate and cover it with a
diagnostic test.
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -3355,6 +3355,30 @@ void
MetaServiceImpl::get_rowset(::google::protobuf::RpcController* controller,
}
VLOG_DEBUG << "tablet_id=" << tablet_id << " stats=" <<
proto_to_json(tablet_stat);
+ if (request->need_alter_job_info()) {
+ // The requester caches this tablet in NOT_READY state. Report
whether the
+ // tablet still has an active schema change job, so that BE can
tell an
+ // in-progress schema change new tablet from an abandoned shadow
tablet
+ // whose job has been cancelled or removed. The schema change job
is
+ // registered under the new tablet's own job key when the job
starts and
+ // cleared from it when the job is committed or aborted.
+ auto job_key = job_tablet_key(
+ {instance_id, idx.table_id(), idx.index_id(),
idx.partition_id(), tablet_id});
+ std::string job_val;
+ TxnErrorCode job_err = txn->get(job_key, &job_val);
+ if (job_err != TxnErrorCode::TXN_OK && job_err !=
TxnErrorCode::TXN_KEY_NOT_FOUND) {
+ code = cast_as<ErrCategory::READ>(job_err);
+ msg = fmt::format("failed to get tablet job, tablet_id={},
err={}", tablet_id,
+ job_err);
+ LOG(WARNING) << msg;
+ return;
+ }
+ TabletJobInfoPB job_pb;
+ response->set_has_alter_job(job_err == TxnErrorCode::TXN_OK &&
+ job_pb.ParseFromString(job_val) &&
+ job_pb.has_schema_change());
Review Comment:
[P2] Check dropped shadows before trusting the mirrored job record.
cancelImpl makes the table NORMAL and the job CANCELLED before onCancel
finishes its drop-index/ABORT cleanup. If another schema change registers while
that cleanup is delayed, start_schema_change_job overwrites the base job but
leaves the previous shadow's mirrored job intact; the old ABORT then fails the
job-ID check, and removeSchemaChangeJob only logs that result. This cancelled
NOTREADY shadow has a dropped index but still returns has_alter_job=true here,
so it continues feeding scores and appearing in the HTTP list until recycling.
Include the shadow's dropped state (or verify/clear stale mirrors) and cover
cancellation followed by a different shadow job.
--
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]
