You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/05/06 04:00:16 UTC
[GitHub] [incubator-doris] pengxiangyu commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
pengxiangyu commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r866443316
##########
be/src/olap/tablet.cpp:
##########
@@ -235,15 +235,6 @@ Status Tablet::add_rowset(RowsetSharedPtr rowset, bool need_persist) {
}
std::vector<RowsetSharedPtr> empty_vec;
modify_rowsets(empty_vec, rowsets_to_delete);
-
Review Comment:
Why do you remove persist here?
##########
be/src/olap/tablet.cpp:
##########
@@ -1446,4 +1437,99 @@ void Tablet::reset_compaction(CompactionType compaction_type) {
}
}
+Status Tablet::create_initial_rowset(const int64_t req_version) {
+ Status res = Status::OK();
+ if (req_version < 1) {
+ LOG(WARNING) << "init version of tablet should at least 1. req.ver=" << req_version;
+ return Status::OLAPInternalError(OLAP_ERR_CE_CMD_PARAMS_ERROR);
+ }
+ Version version(0, req_version);
+ RowsetSharedPtr new_rowset;
+ do {
+ // there is no data in init rowset, so overlapping info is unknown.
+ std::unique_ptr<RowsetWriter> rs_writer;
+ res = create_rowset_writer(version, VISIBLE, OVERLAP_UNKNOWN, &rs_writer);
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to init rowset writer for tablet " << full_name();
+ break;
+ }
+ res = rs_writer->flush();
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to flush rowset writer for tablet " << full_name();
+ break;
+ }
+
+ new_rowset = rs_writer->build();
+ res = add_rowset(new_rowset);
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to add rowset for tablet " << full_name();
+ break;
+ }
+ } while (0);
+
+ // Unregister index and delete files(index and data) if failed
+ if (!res.ok()) {
+ LOG(WARNING) << "fail to create initial rowset. res=" << res << " version=" << req_version;
+ StorageEngine::instance()->add_unused_rowset(new_rowset);
+ return res;
+ }
+ set_cumulative_layer_point(req_version + 1);
+ return res;
+}
+
+Status Tablet::create_rowset_writer(const Version& version, const RowsetStatePB& rowset_state,
Review Comment:
The two function of create_rowset_writer can be one. Too many duplicate code here.
##########
be/src/olap/tablet_manager.cpp:
##########
@@ -1272,11 +1185,19 @@ Status TabletManager::_create_tablet_meta_unlocked(const TCreateTabletReq& reque
RETURN_NOT_OK_LOG(store->get_shard(&shard_id), "fail to get root path shard");
Status res = TabletMeta::create(request, TabletUid::gen_uid(), shard_id, next_unique_id,
col_idx_to_unique_id, tablet_meta);
-
- if (request.__isset.storage_format && request.storage_format != TStorageFormat::V1) {
- (*tablet_meta)->set_preferred_rowset_type(BETA_ROWSET);
- } else {
- (*tablet_meta)->set_preferred_rowset_type(ALPHA_ROWSET);
+ RETURN_NOT_OK(res);
+ if (request.__isset.storage_format) {
Review Comment:
check code in create_rowset_writer(), if storage_format is not set, preferred_rowset_type will be null, but it is used in create_rowset_writer(), it will make an unpredictable error.
##########
be/src/olap/tablet.cpp:
##########
@@ -1446,4 +1437,99 @@ void Tablet::reset_compaction(CompactionType compaction_type) {
}
}
+Status Tablet::create_initial_rowset(const int64_t req_version) {
+ Status res = Status::OK();
+ if (req_version < 1) {
+ LOG(WARNING) << "init version of tablet should at least 1. req.ver=" << req_version;
+ return Status::OLAPInternalError(OLAP_ERR_CE_CMD_PARAMS_ERROR);
+ }
+ Version version(0, req_version);
+ RowsetSharedPtr new_rowset;
+ do {
+ // there is no data in init rowset, so overlapping info is unknown.
+ std::unique_ptr<RowsetWriter> rs_writer;
+ res = create_rowset_writer(version, VISIBLE, OVERLAP_UNKNOWN, &rs_writer);
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to init rowset writer for tablet " << full_name();
+ break;
+ }
+ res = rs_writer->flush();
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to flush rowset writer for tablet " << full_name();
+ break;
+ }
+
+ new_rowset = rs_writer->build();
+ res = add_rowset(new_rowset);
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to add rowset for tablet " << full_name();
+ break;
+ }
+ } while (0);
+
+ // Unregister index and delete files(index and data) if failed
+ if (!res.ok()) {
+ LOG(WARNING) << "fail to create initial rowset. res=" << res << " version=" << req_version;
+ StorageEngine::instance()->add_unused_rowset(new_rowset);
+ return res;
+ }
+ set_cumulative_layer_point(req_version + 1);
+ return res;
+}
+
+Status Tablet::create_rowset_writer(const Version& version, const RowsetStatePB& rowset_state,
Review Comment:
The two function of create_rowset_writer can be one. Too many duplicate code here.
##########
be/src/olap/tablet_manager.cpp:
##########
@@ -1272,11 +1185,19 @@ Status TabletManager::_create_tablet_meta_unlocked(const TCreateTabletReq& reque
RETURN_NOT_OK_LOG(store->get_shard(&shard_id), "fail to get root path shard");
Status res = TabletMeta::create(request, TabletUid::gen_uid(), shard_id, next_unique_id,
col_idx_to_unique_id, tablet_meta);
-
- if (request.__isset.storage_format && request.storage_format != TStorageFormat::V1) {
- (*tablet_meta)->set_preferred_rowset_type(BETA_ROWSET);
- } else {
- (*tablet_meta)->set_preferred_rowset_type(ALPHA_ROWSET);
+ RETURN_NOT_OK(res);
+ if (request.__isset.storage_format) {
Review Comment:
check code in create_rowset_writer(), if storage_format is not set, preferred_rowset_type will be null, but it is used in create_rowset_writer(), it will make an unpredictable error.
##########
be/src/olap/tablet.cpp:
##########
@@ -1446,4 +1437,99 @@ void Tablet::reset_compaction(CompactionType compaction_type) {
}
}
+Status Tablet::create_initial_rowset(const int64_t req_version) {
+ Status res = Status::OK();
+ if (req_version < 1) {
+ LOG(WARNING) << "init version of tablet should at least 1. req.ver=" << req_version;
+ return Status::OLAPInternalError(OLAP_ERR_CE_CMD_PARAMS_ERROR);
+ }
+ Version version(0, req_version);
+ RowsetSharedPtr new_rowset;
+ do {
+ // there is no data in init rowset, so overlapping info is unknown.
+ std::unique_ptr<RowsetWriter> rs_writer;
+ res = create_rowset_writer(version, VISIBLE, OVERLAP_UNKNOWN, &rs_writer);
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to init rowset writer for tablet " << full_name();
+ break;
+ }
+ res = rs_writer->flush();
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to flush rowset writer for tablet " << full_name();
+ break;
+ }
+
+ new_rowset = rs_writer->build();
+ res = add_rowset(new_rowset);
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to add rowset for tablet " << full_name();
+ break;
+ }
+ } while (0);
+
+ // Unregister index and delete files(index and data) if failed
+ if (!res.ok()) {
+ LOG(WARNING) << "fail to create initial rowset. res=" << res << " version=" << req_version;
+ StorageEngine::instance()->add_unused_rowset(new_rowset);
+ return res;
+ }
+ set_cumulative_layer_point(req_version + 1);
+ return res;
+}
+
+Status Tablet::create_rowset_writer(const Version& version, const RowsetStatePB& rowset_state,
+ const SegmentsOverlapPB& overlap,
+ std::unique_ptr<RowsetWriter>* rowset_writer) {
+ RowsetWriterContext context;
+ context.rowset_id = StorageEngine::instance()->next_rowset_id();
+ context.tablet_uid = tablet_uid();
+ context.tablet_id = tablet_id();
+ context.partition_id = partition_id();
+ context.tablet_schema_hash = schema_hash();
+ context.data_dir = data_dir();
+ context.rowset_type = tablet_meta()->preferred_rowset_type();
+ // Alpha Rowset will be removed in the future, so that if the tablet's default rowset type is
+ // alpah rowset, then set the newly created rowset to storage engine's default rowset.
+ if (context.rowset_type == ALPHA_ROWSET) {
+ context.rowset_type = StorageEngine::instance()->default_rowset_type();
+ }
+ context.path_desc = tablet_path_desc();
+ context.tablet_schema = &(tablet_schema());
+ context.rowset_state = rowset_state;
+ context.version = version;
+ context.segments_overlap = overlap;
+ // The test results show that one rs writer is low-memory-footprint, there is no need to tracker its mem pool
+ return RowsetFactory::create_rowset_writer(context, rowset_writer);
+}
+
+Status Tablet::create_rowset_writer(const int64_t& txn_id, const PUniqueId& load_id,
+ const RowsetStatePB& rowset_state,
+ const SegmentsOverlapPB& overlap,
+ std::unique_ptr<RowsetWriter>* rowset_writer) {
+ RowsetWriterContext context;
+ context.rowset_id = StorageEngine::instance()->next_rowset_id();
+ context.tablet_uid = tablet_uid();
+ context.tablet_id = tablet_id();
+ context.partition_id = partition_id();
+ context.tablet_schema_hash = schema_hash();
+ context.rowset_type = tablet_meta()->preferred_rowset_type();
Review Comment:
if not set preferred_rowset_type, it may be nullptr, it is unsafety.
##########
be/src/olap/tablet.cpp:
##########
@@ -1446,4 +1437,99 @@ void Tablet::reset_compaction(CompactionType compaction_type) {
}
}
+Status Tablet::create_initial_rowset(const int64_t req_version) {
+ Status res = Status::OK();
+ if (req_version < 1) {
+ LOG(WARNING) << "init version of tablet should at least 1. req.ver=" << req_version;
+ return Status::OLAPInternalError(OLAP_ERR_CE_CMD_PARAMS_ERROR);
+ }
+ Version version(0, req_version);
+ RowsetSharedPtr new_rowset;
+ do {
+ // there is no data in init rowset, so overlapping info is unknown.
+ std::unique_ptr<RowsetWriter> rs_writer;
+ res = create_rowset_writer(version, VISIBLE, OVERLAP_UNKNOWN, &rs_writer);
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to init rowset writer for tablet " << full_name();
+ break;
+ }
+ res = rs_writer->flush();
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to flush rowset writer for tablet " << full_name();
+ break;
+ }
+
+ new_rowset = rs_writer->build();
+ res = add_rowset(new_rowset);
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to add rowset for tablet " << full_name();
+ break;
+ }
+ } while (0);
+
+ // Unregister index and delete files(index and data) if failed
+ if (!res.ok()) {
+ LOG(WARNING) << "fail to create initial rowset. res=" << res << " version=" << req_version;
+ StorageEngine::instance()->add_unused_rowset(new_rowset);
+ return res;
+ }
+ set_cumulative_layer_point(req_version + 1);
+ return res;
+}
+
+Status Tablet::create_rowset_writer(const Version& version, const RowsetStatePB& rowset_state,
+ const SegmentsOverlapPB& overlap,
+ std::unique_ptr<RowsetWriter>* rowset_writer) {
+ RowsetWriterContext context;
+ context.rowset_id = StorageEngine::instance()->next_rowset_id();
+ context.tablet_uid = tablet_uid();
+ context.tablet_id = tablet_id();
+ context.partition_id = partition_id();
+ context.tablet_schema_hash = schema_hash();
+ context.data_dir = data_dir();
+ context.rowset_type = tablet_meta()->preferred_rowset_type();
+ // Alpha Rowset will be removed in the future, so that if the tablet's default rowset type is
+ // alpah rowset, then set the newly created rowset to storage engine's default rowset.
+ if (context.rowset_type == ALPHA_ROWSET) {
+ context.rowset_type = StorageEngine::instance()->default_rowset_type();
+ }
+ context.path_desc = tablet_path_desc();
+ context.tablet_schema = &(tablet_schema());
+ context.rowset_state = rowset_state;
+ context.version = version;
+ context.segments_overlap = overlap;
+ // The test results show that one rs writer is low-memory-footprint, there is no need to tracker its mem pool
+ return RowsetFactory::create_rowset_writer(context, rowset_writer);
+}
+
+Status Tablet::create_rowset_writer(const int64_t& txn_id, const PUniqueId& load_id,
+ const RowsetStatePB& rowset_state,
+ const SegmentsOverlapPB& overlap,
+ std::unique_ptr<RowsetWriter>* rowset_writer) {
+ RowsetWriterContext context;
+ context.rowset_id = StorageEngine::instance()->next_rowset_id();
+ context.tablet_uid = tablet_uid();
+ context.tablet_id = tablet_id();
+ context.partition_id = partition_id();
+ context.tablet_schema_hash = schema_hash();
+ context.rowset_type = tablet_meta()->preferred_rowset_type();
Review Comment:
if not set preferred_rowset_type, it may be nullptr, it is unsafety.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org