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/05 03:50:14 UTC
[GitHub] [incubator-doris] yiguolei opened a new pull request, #9368: [refactor][rowset]move rowset writer to a single place
yiguolei opened a new pull request, #9368:
URL: https://github.com/apache/incubator-doris/pull/9368
Refactor rowset created logic. Move create rowset writer logic to tablet because most of these logic are related with the specific 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: 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
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#issuecomment-1124599169
PR approved by at least one committer and no changes requested.
--
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
[GitHub] [incubator-doris] pengxiangyu commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
pengxiangyu commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r867301624
##########
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:
Duplicated code is not elegant，I mean you can do like this:
Status Tablet::create_rowset_writer(const Version& version ...
RowsetWriterContext context = _init_context();
context.version = version;
...
}
Status Tablet::create_rowset_writer(const int64_t& txn_id, const PUniqueId& load_id...
RowsetWriterContext context = _init_context();
context.txn_id = txn_id;
context.load_id = load_id;
...
}
RowsetWriterContext _init_context() {
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.segments_overlap = overlap;
return context;
}
--
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
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#issuecomment-1127458986
PR approved by at least one committer and no changes requested.
--
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
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r866533398
##########
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:
It could not be one method, because one method is for load purpose so it have load id and txn id, and the other method is for existing rowset so it have version.
Although we could combine there 2 method to a single method, but has to add a boolean variable to indicate which variable is useful and user has to set default value during call this method. I am not preferred with this way.
--
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
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r866533398
##########
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:
It could not be one method, because one method is for load purpose so it have load id and txn id, and the other method is for existing rowset so it have version.
Although we could combine these 2 method to a single method, but has to add a boolean variable to indicate which variable is useful and user has to set default value during call this method. I am not preferred with this way.
--
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
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#issuecomment-1123606288
PR approved by anyone and no changes requested.
--
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
[GitHub] [incubator-doris] yiguolei merged pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368
--
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
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r867298944
##########
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:
nee_persist is always false
--
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
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r866534547
##########
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:
RowsetTypePB _preferred_rowset_type = BETA_ROWSET; in tablet_meta.h
##########
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:
RowsetTypePB _preferred_rowset_type = BETA_ROWSET; in tablet_meta.h
--
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
[GitHub] [incubator-doris] pengxiangyu commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
pengxiangyu commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r866443317
##########
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:
But if a new member is added into RowsetWriterContext, you have to added in the two function, it's better to collect the same lines in a private function. Such as _init_context();
--
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
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r866531867
##########
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:
because all methods use nee_persist= false.
--
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
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r866535080
##########
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:
If storage format is not set, then _preferred_rowset_type == BETA_ROWSET according to the definition in
RowsetTypePB _preferred_rowset_type = BETA_ROWSET; in tablet_meta.h
--
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
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r868777978
##########
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:
I see, thanks. I will modify it.
--
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
[GitHub] [incubator-doris] pengxiangyu commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9368: [refactor][rowset]move rowset writer to a single place
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9368:
URL: https://github.com/apache/incubator-doris/pull/9368#discussion_r867299163
##########
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:
I think it is affordable. And it is better than add a boolean variable to indicate which method to use because it need modify too many invoking place and the method is not very clear.
--
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