Skip to content

Commit 5899a2a

Browse files
committed
[Refactor] Deprecate lake tablet_row_nums; derive first-load row counts from tablet_stats
Mark PublishVersionResponse.tablet_row_nums (ordinal 4) as deprecated and stop populating it. BE now reports first-import per-tablet stats through the newer tablet_stats field: the publish gate broadens to emit stats for any tablet on first import (base_version == 1) in addition to every range-distribution tablet, computed via compute_tablet_stats without delvec I/O on the publish hot path. FE derives first-load row counts from tablet_stats[*].num_rows via collectFirstLoadRowCounts, replacing reads of the deprecated map. The aggregate-publish merge of tablet_row_nums is removed. For primary-key tablets the first-load row count is now the live-row count (num_rows - num_dels) rather than the raw rowset sum the deprecated field reported; this is more accurate and is self-healed by the periodic TabletStatMgr scan. The field and ordinal are retained ([deprecated = true]) for wire compatibility during a BE-before-FE rolling upgrade. Signed-off-by: xiangguangyxg <xiangguangyxg@gmail.com>
1 parent bd971dd commit 5899a2a

5 files changed

Lines changed: 101 additions & 32 deletions

File tree

be/src/service/service_be/lake_service.cpp

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -421,13 +421,18 @@ void LakeServiceImpl::publish_version(::google::protobuf::RpcController* control
421421
if (res.ok()) {
422422
auto metadata = std::move(res).value();
423423
auto score = compaction_score(_tablet_mgr, metadata);
424-
// Per-tablet stats for range-distribution tablets, fed back to FE for real-time
425-
// split/merge triggering. Summed without any delvec I/O (publish hot path).
426-
const bool has_range_stats = metadata->has_range();
427-
int64_t range_num_rows = 0;
428-
int64_t range_data_size = 0;
429-
if (has_range_stats) {
430-
compute_tablet_stats(*metadata, &range_num_rows, &range_data_size);
424+
// Per-tablet stats returned to FE: for range-distribution tablets (every
425+
// publish) to drive real-time split/merge, and on first import (base_version==1)
426+
// for any tablet so FE can collect first-load statistics. Supersedes the
427+
// deprecated tablet_row_nums field; note num_rows here is the live-row count
428+
// (PK tablets subtract deletes) rather than that field's raw rowset row sum,
429+
// so a first load with intra-batch duplicate keys or deletes reports fewer
430+
// rows. Summed without delvec I/O (publish hot path).
431+
const bool emit_stats = metadata->has_range() || request->base_version() == 1;
432+
int64_t stats_num_rows = 0;
433+
int64_t stats_data_size = 0;
434+
if (emit_stats) {
435+
compute_tablet_stats(*metadata, &stats_num_rows, &stats_data_size);
431436
}
432437
// Copy metadata out of the lock(response_mtx), to let it execute in parallel.
433438
TabletMetadataPB local_metadata;
@@ -437,17 +442,10 @@ void LakeServiceImpl::publish_version(::google::protobuf::RpcController* control
437442
{
438443
std::lock_guard l(response_mtx);
439444
response->mutable_compaction_scores()->insert({metadata->id(), score});
440-
if (request->base_version() == 1) {
441-
int64_t row_nums = std::accumulate(
442-
metadata->rowsets().begin(), metadata->rowsets().end(), 0,
443-
[](int64_t sum, const auto& rowset) { return sum + rowset.num_rows(); });
444-
// Used to collect statistics when the partition is first imported
445-
response->mutable_tablet_row_nums()->insert({metadata->id(), row_nums});
446-
}
447-
if (has_range_stats) {
445+
if (emit_stats) {
448446
auto* stat = &(*response->mutable_tablet_stats())[metadata->id()];
449-
stat->set_num_rows(range_num_rows);
450-
stat->set_data_size(range_data_size);
447+
stat->set_num_rows(stats_num_rows);
448+
stat->set_data_size(stats_data_size);
451449
}
452450
if (skip_write_tablet_metadata) {
453451
(*response->mutable_tablet_metas())[metadata->id()].Swap(&local_metadata);
@@ -664,9 +662,6 @@ struct AggregatePublishContext {
664662
for (const auto& [tid, score] : resp->compaction_scores()) {
665663
(*response->mutable_compaction_scores())[tid] = score;
666664
}
667-
for (const auto& [tid, row_num] : resp->tablet_row_nums()) {
668-
(*response->mutable_tablet_row_nums())[tid] = row_num;
669-
}
670665
for (const auto& [tid, stat] : resp->tablet_stats()) {
671666
(*response->mutable_tablet_stats())[tid] = stat;
672667
}

be/test/service/lake_service_test.cpp

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6038,10 +6038,10 @@ TEST_F(LakeServiceTest, test_compute_tablet_stats) {
60386038
// - A non-range tablet produces no tablet_stats entry.
60396039
// - A range-distribution tablet (metadata has_range() == true) produces a
60406040
// tablet_stats entry with data_size > 0.
6041-
TEST_F(LakeServiceTest, test_publish_returns_tablet_stats_for_range_tablet) {
6042-
// --- Negative: non-range tablet must NOT appear in tablet_stats ---
6041+
TEST_F(LakeServiceTest, test_publish_returns_tablet_stats) {
6042+
// --- First load (base_version==1) of a non-range tablet: stats are now reported via
6043+
// tablet_stats; the deprecated tablet_row_nums field is no longer populated. ---
60436044
{
6044-
// Publish a txn on the default tablet (no range set).
60456045
TxnLog log = generate_write_txn_log(2, 101, 4096);
60466046
ASSERT_OK(_tablet_mgr->put_txn_log(log));
60476047

@@ -6055,7 +6055,30 @@ TEST_F(LakeServiceTest, test_publish_returns_tablet_stats_for_range_tablet) {
60556055
_lake_service.publish_version(nullptr, &request, &response, nullptr);
60566056

60576057
ASSERT_EQ(0, response.failed_tablets_size()) << response.status().error_msgs(0);
6058-
EXPECT_EQ(0, response.tablet_stats().count(_tablet_id)) << "non-range tablet must not appear in tablet_stats";
6058+
auto it = response.tablet_stats().find(_tablet_id);
6059+
ASSERT_NE(it, response.tablet_stats().end()) << "first-load tablet must report stats via tablet_stats";
6060+
EXPECT_EQ(101, it->second.num_rows()) << "first-load row count must flow through tablet_stats";
6061+
EXPECT_EQ(0, response.tablet_row_nums().count(_tablet_id))
6062+
<< "deprecated tablet_row_nums must no longer be populated";
6063+
}
6064+
6065+
// --- Non-first-load (base_version>1) of a non-range tablet: must NOT appear in tablet_stats. ---
6066+
{
6067+
TxnLog log = generate_write_txn_log(1, 50, 2048);
6068+
ASSERT_OK(_tablet_mgr->put_txn_log(log));
6069+
6070+
PublishVersionRequest request;
6071+
request.set_base_version(2);
6072+
request.set_new_version(3);
6073+
request.add_tablet_ids(_tablet_id);
6074+
request.add_txn_ids(log.txn_id());
6075+
6076+
PublishVersionResponse response;
6077+
_lake_service.publish_version(nullptr, &request, &response, nullptr);
6078+
6079+
ASSERT_EQ(0, response.failed_tablets_size()) << response.status().error_msgs(0);
6080+
EXPECT_EQ(0, response.tablet_stats().count(_tablet_id))
6081+
<< "non-range, non-first-load tablet must not appear in tablet_stats";
60596082
}
60606083

60616084
// --- Positive: range-distribution tablet MUST appear in tablet_stats ---

fe/fe-core/src/main/java/com/starrocks/lake/Utils.java

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.starrocks.lake;
1616

17+
import com.google.common.annotations.VisibleForTesting;
1718
import com.google.common.collect.Lists;
1819
import com.staros.proto.ShardInfo;
1920
import com.starrocks.alter.reshard.PublishTabletsInfo;
@@ -179,6 +180,21 @@ public static boolean noOpPublishForForceSkip(long jobId, String reason, long wa
179180
}
180181
}
181182

183+
// Derive first-import (base_version==1) per-tablet row counts from the publish response's
184+
// tablet_stats, now that the tablet_row_nums field is deprecated. BE populates tablet_stats
185+
// for every first-load tablet (and every range-distribution tablet).
186+
@VisibleForTesting
187+
static void collectFirstLoadRowCounts(PublishVersionResponse response, Map<Long, Long> tabletRowNum) {
188+
if (tabletRowNum == null || response == null || response.tabletStats == null) {
189+
return;
190+
}
191+
response.tabletStats.forEach((tabletId, stat) -> {
192+
if (stat != null && stat.numRows != null) {
193+
tabletRowNum.put(tabletId, stat.numRows);
194+
}
195+
});
196+
}
197+
182198
public static void publishVersionBatch(@NotNull List<Tablet> tablets, List<TxnInfoPB> txnInfos,
183199
long baseVersion, long newVersion,
184200
Map<Long, Double> compactionScores,
@@ -256,8 +272,8 @@ public static void publishVersionBatch(@NotNull List<Tablet> tablets, List<TxnIn
256272
tabletRanges.put(entry.getKey(), TabletRange.fromProto(entry.getValue()));
257273
}
258274
}
259-
if (baseVersion == 1 && tabletRowNum != null && response != null && response.tabletRowNums != null) {
260-
tabletRowNum.putAll(response.tabletRowNums);
275+
if (baseVersion == 1) {
276+
collectFirstLoadRowCounts(response, tabletRowNum);
261277
}
262278
if (tabletStats != null && response != null && response.tabletStats != null) {
263279
tabletStats.putAll(response.tabletStats);
@@ -487,8 +503,8 @@ public static void sendAggregatePublishVersionRequest(AggregatePublishVersionReq
487503
tabletRanges.put(entry.getKey(), TabletRange.fromProto(entry.getValue()));
488504
}
489505
}
490-
if (baseVersion == 1 && tabletRowNum != null && response != null && response.tabletRowNums != null) {
491-
tabletRowNum.putAll(response.tabletRowNums);
506+
if (baseVersion == 1) {
507+
collectFirstLoadRowCounts(response, tabletRowNum);
492508
}
493509
if (tabletStats != null && response != null && response.tabletStats != null) {
494510
tabletStats.putAll(response.tabletStats);

fe/fe-core/src/test/java/com/starrocks/lake/UtilsTest.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
package com.starrocks.lake;
1717

1818
import com.starrocks.common.StarRocksException;
19+
import com.starrocks.proto.PublishVersionResponse;
20+
import com.starrocks.proto.TabletStatPB;
1921
import com.starrocks.server.GlobalStateMgr;
2022
import com.starrocks.server.NodeMgr;
2123
import com.starrocks.server.WarehouseManager;
@@ -92,4 +94,33 @@ public void testGetWarehouseIdByNodeId() {
9294
Assertions.assertEquals(10001L, Utils.getWarehouseIdByNodeId(systemInfo, 10001).get().longValue());
9395
Assertions.assertEquals(10002L, Utils.getWarehouseIdByNodeId(systemInfo, 10002).get().longValue());
9496
}
97+
98+
@Test
99+
public void testCollectFirstLoadRowCounts() {
100+
java.util.Map<Long, Long> out = new java.util.HashMap<>();
101+
102+
// null response / null tablet_stats / null out-param -> no-op, no NPE
103+
Utils.collectFirstLoadRowCounts(null, out);
104+
PublishVersionResponse response = new PublishVersionResponse();
105+
Utils.collectFirstLoadRowCounts(response, out);
106+
Assertions.assertTrue(out.isEmpty());
107+
108+
// populated -> derive numRows; skip entries whose numRows is null
109+
response.tabletStats = new java.util.HashMap<>();
110+
TabletStatPB withRows = new TabletStatPB();
111+
withRows.numRows = 7L;
112+
withRows.dataSize = 70L;
113+
TabletStatPB noRows = new TabletStatPB();
114+
noRows.dataSize = 80L; // numRows left null -> must be skipped
115+
response.tabletStats.put(11L, withRows);
116+
response.tabletStats.put(22L, noRows);
117+
118+
Utils.collectFirstLoadRowCounts(response, out);
119+
Assertions.assertEquals(1, out.size());
120+
Assertions.assertEquals(Long.valueOf(7L), out.get(11L));
121+
Assertions.assertNull(out.get(22L));
122+
123+
// null out-param -> no-op, no NPE
124+
Utils.collectFirstLoadRowCounts(response, null);
125+
}
95126
}

gensrc/proto/lake_service.proto

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,20 @@ message PublishVersionResponse {
6060
// Mapping from tablet id to compaction score.
6161
map<int64, double> compaction_scores = 2;
6262
optional StatusPB status = 3;
63-
// Mapping from tablet id to row_nums when the partition is first imported
64-
map<int64, int64> tablet_row_nums = 4;
63+
// Deprecated: first-import row counts are now derived from |tablet_stats| (field 8),
64+
// which BE also populates on first load. BE no longer writes this field; kept for
65+
// wire compatibility during a BE-before-FE rolling upgrade. Do not reuse ordinal 4.
66+
map<int64, int64> tablet_row_nums = 4 [deprecated = true];
6567
map<int64, TabletMetadataPB> tablet_metas = 5;
6668
// New tablet ranges for tablet splitting
6769
map<int64, TabletRangePB> tablet_ranges = 6;
6870
// Tablets that have async vector indexes and were published in this batch.
6971
// FE uses this to register new pending build tasks.
7072
repeated VectorIndexBuildInfoPB vector_index_build_infos = 7;
71-
// Per-tablet aggregate stats for range-distribution tablets, returned on every
72-
// publish so FE can trigger split/merge in real time. Empty for non-range tablets.
73+
// Per-tablet aggregate stats. Returned for range-distribution tablets on every
74+
// publish so FE can trigger split/merge in real time, and for any tablet on first
75+
// import (base_version == 1) so FE can collect first-load statistics (supersedes the
76+
// deprecated tablet_row_nums field). Empty for non-range tablets after first import.
7377
map<int64, TabletStatPB> tablet_stats = 8;
7478
}
7579

0 commit comments

Comments
 (0)