github-actions[bot] commented on code in PR #68198:
URL: https://github.com/apache/doris/pull/68198#discussion_r4046293607
##########
be/test/storage/compaction/segcompaction_test.cpp:
##########
@@ -292,6 +299,139 @@ class SegCompactionTest : public testing::Test {
EXPECT_EQ(Status::OK(), s);
}
+ // ---- variant / segment-compaction interaction ----
+
+ static constexpr int kVariantSegments = 12;
+ static constexpr int kVariantRowsPerSegment = 200;
+
+ // Each segment carries a key of its own ("s<seg>") on top of the shared
ones, so a merge
+ // would have to re-split subcolumns and sparse paths rather than copy the
input layout.
+ static std::string variant_json(int segment, int rid) {
+ return fmt::format(R"({{"a":{},"b":"mark_{}_{}","s{}":{}}})", rid,
segment, rid, segment,
+ rid * 7);
+ }
+
+ // (c1 INT key, v VARIANT or INT). The variant flavour is the one
BetaRowsetWriter opts out
+ // of segment compaction.
+ TabletSchemaSPtr create_variant_tablet_schema(bool with_variant) {
+ TabletSchemaPB schema_pb;
+ schema_pb.set_keys_type(KeysType::DUP_KEYS);
+ schema_pb.set_num_short_key_columns(1);
+ schema_pb.set_num_rows_per_row_block(1024);
+ schema_pb.set_compress_kind(COMPRESS_NONE);
+ schema_pb.set_next_column_unique_id(3);
+
+ ColumnPB* key = schema_pb.add_column();
+ key->set_unique_id(1);
+ key->set_name("c1");
+ key->set_type("INT");
+ key->set_is_key(true);
+ key->set_length(4);
+ key->set_index_length(4);
+ key->set_is_nullable(false);
+
+ ColumnPB* value = schema_pb.add_column();
+ value->set_unique_id(2);
+ value->set_name("v");
+ value->set_is_key(false);
+ value->set_is_nullable(true);
+ if (with_variant) {
+ value->set_type("VARIANT");
+ // Small enough that only some paths stay extracted; the rest go
to the sparse column.
+ value->set_variant_max_subcolumns_count(3);
+ value->set_variant_max_sparse_column_statistics_size(10000);
+ value->set_variant_sparse_hash_shard_count(1);
+ } else {
+ value->set_type("INT");
+ value->set_length(4);
+ }
+
+ auto tablet_schema = std::make_shared<TabletSchema>();
+ tablet_schema->init_from_pb(schema_pb);
+ return tablet_schema;
+ }
+
+ // `wait_for_segcompaction` sleeps between flushes so the async worker
gets to start:
+ // BetaRowsetWriter::_close_file_writers cancels a task that has not
started yet, which would
+ // leave a rowset uncompacted for timing reasons rather than for the
reason under test.
+ void write_variant_rowset(int64_t id, const TabletSchemaSPtr&
tablet_schema,
+ bool wait_for_segcompaction, RowsetSharedPtr*
rowset) {
+ RowsetWriterContext writer_context;
+ create_rowset_writer_context(id, tablet_schema, &writer_context);
+ auto res = RowsetFactory::create_rowset_writer(*l_engine,
writer_context, false);
+ ASSERT_TRUE(res.has_value()) << res.error();
+ auto rowset_writer = std::move(res).value();
+
+ const bool with_variant =
+ tablet_schema->column(1).type() ==
FieldType::OLAP_FIELD_TYPE_VARIANT;
+ for (int seg = 0; seg < kVariantSegments; ++seg) {
+ Block block = tablet_schema->create_storage_block();
+ auto columns = std::move(block).mutate_columns();
+ auto raw_json = ColumnString::create();
+ auto* nullable = assert_cast<ColumnNullable*>(columns[1].get());
+ for (int rid = 0; rid < kVariantRowsPerSegment; ++rid) {
+ int32_t c1 = seg * kVariantRowsPerSegment + rid;
+ columns[0]->insert_data(reinterpret_cast<const char*>(&c1),
sizeof(c1));
+ if (with_variant) {
+ std::string json = variant_json(seg, rid);
+ raw_json->insert_data(json.data(), json.size());
+ } else {
+
nullable->get_nested_column().insert_data(reinterpret_cast<const char*>(&c1),
+ sizeof(c1));
+ }
+ nullable->get_null_map_data().push_back(0);
+ }
+ if (with_variant) {
+ VariantUtil::insert_json_rows(
+
assert_cast<ColumnVariantV2&>(nullable->get_nested_column()), *raw_json);
+ }
+ ASSERT_TRUE(add_block_with_columns(rowset_writer.get(), &block,
&columns).ok());
+ ASSERT_TRUE(rowset_writer->flush().ok());
+ if (wait_for_segcompaction) {
+ sleep(1);
Review Comment:
[P2] Synchronize with segcompaction instead of sleeping
This helper sleeps after all 12 flushes, so the new test adds roughly 12
seconds even though no task can be submitted before the fifth segment. It is
still timing-dependent: `build()` cancels a queued task if the worker has not
claimed it yet, so a busy runner can leave the control rowset unmerged and fail
`EXPECT_LT` even when segcompaction is correct. Please wait on a sync
point/condition that proves the worker started or completed instead of using
wall-clock sleeps.
--
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]
