lingbin commented on a change in pull request #2918: Some refactor on
`TabletManager`
URL: https://github.com/apache/incubator-doris/pull/2918#discussion_r380005093
##########
File path: be/src/olap/tablet_manager.cpp
##########
@@ -212,120 +191,133 @@ bool
TabletManager::_check_tablet_id_exist_unlocked(TTabletId tablet_id) {
return it != _tablet_map.end() && !it->second.table_arr.empty();
}
-void TabletManager::clear() {
- _tablet_map.clear();
- _shutdown_tablets.clear();
-}
-
OLAPStatus TabletManager::create_tablet(const TCreateTabletReq& request,
std::vector<DataDir*> stores) {
- LOG(INFO) << "begin to process create tablet. tablet=" << request.tablet_id
- << ", schema_hash=" << request.tablet_schema.schema_hash;
- WriteLock wrlock(&_tablet_map_lock);
- OLAPStatus res = OLAP_SUCCESS;
DorisMetrics::create_tablet_requests_total.increment(1);
- // Make sure create_tablet operation is idempotent:
- // return true if tablet with same tablet_id and schema_hash exist,
- // false if tablet with same tablet_id but different schema_hash
exist
- // during alter, if the tablet(same tabletid and schema hash) already exist
- // then just return true, if tablet id with different schema hash exist,
wait report
- // task to delete the tablet
- if (_check_tablet_id_exist_unlocked(request.tablet_id)) {
- TabletSharedPtr tablet = _get_tablet_unlocked(
- request.tablet_id, request.tablet_schema.schema_hash);
+
+ int64_t tablet_id = request.tablet_id;
+ int32_t schema_hash = request.tablet_schema.schema_hash;
+ LOG(INFO) << "begin to create tablet. tablet_id=" << tablet_id
+ << ", schema_hash=" << schema_hash;
+
+ WriteLock wrlock(&_tablet_map_lock);
+ // Make create_tablet operation to be idempotent:
+ // 1. Return true if tablet with same tablet_id and schema_hash exist;
+ // false if tablet with same tablet_id but different schema_hash
exist.
+ // 2. When this is an alter task, if the tablet(both tablet_id and
schema_hash are
+ // same) already exist, then just return true(an duplicate request). But if
+ // tablet_id exist but with different schema_hash, return an error(report
task will
+ // eventually trigger its deletion).
+ if (_check_tablet_id_exist_unlocked(tablet_id)) {
+ TabletSharedPtr tablet = _get_tablet_unlocked(tablet_id, schema_hash);
if (tablet != nullptr) {
- LOG(INFO) << "create tablet success because tablet already exist.
tablet_id="
- << request.tablet_id;
+ LOG(INFO) << "success to create tablet. tablet already exist.
tablet_id=" << tablet_id;
return OLAP_SUCCESS;
} else {
- LOG(WARNING) << "tablet with different schema hash already exists.
tablet_id="
- << request.tablet_id;
+ LOG(WARNING) << "fail to create tablet. tablet exist but with
different schema_hash. "
+ << "tablet_id=" << tablet_id << ", schema_hash=" <<
schema_hash;
+ DorisMetrics::create_tablet_requests_failed.increment(1);
return OLAP_ERR_CE_TABLET_ID_EXIST;
}
}
- TabletSharedPtr ref_tablet = nullptr;
- bool is_schema_change_tablet = false;
- // if the CreateTabletReq has base_tablet_id then it is a alter tablet
request
+ TabletSharedPtr base_tablet = nullptr;
+ bool is_schema_change = false;
+ // If the CreateTabletReq has base_tablet_id then it is a alter-tablet
request
if (request.__isset.base_tablet_id && request.base_tablet_id > 0) {
- is_schema_change_tablet = true;
- ref_tablet = _get_tablet_unlocked(request.base_tablet_id,
request.base_schema_hash);
- if (ref_tablet == nullptr) {
- LOG(WARNING) << "fail to create new tablet. new_tablet_id=" <<
request.tablet_id
- << ", new_schema_hash=" <<
request.tablet_schema.schema_hash
- << ", because could not find base tablet,
base_tablet_id=" << request.base_tablet_id
+ is_schema_change = true;
+ base_tablet = _get_tablet_unlocked(request.base_tablet_id,
request.base_schema_hash);
+ if (base_tablet == nullptr) {
+ LOG(WARNING) << "fail to create tablet(change schema), base tablet
does not exist. "
+ << "new_tablet_id=" << tablet_id << ",
new_schema_hash=" << schema_hash
+ << ", base_tablet_id=" << request.base_tablet_id
<< ", base_schema_hash=" << request.base_schema_hash;
+ DorisMetrics::create_tablet_requests_failed.increment(1);
return OLAP_ERR_TABLE_CREATE_META_ERROR;
}
- // schema change should use the same data dir
+ // If we are doing schema-change, we should use the same data dir
+ // TODO(lingbin): A litter trick here, the directory should be
determined before
+ // entering this method
stores.clear();
- stores.push_back(ref_tablet->data_dir());
+ stores.push_back(base_tablet->data_dir());
}
- // set alter type to schema change. it is useless
- TabletSharedPtr tablet =
_internal_create_tablet_unlocked(AlterTabletType::SCHEMA_CHANGE, request,
- is_schema_change_tablet, ref_tablet, stores);
+ // set alter type to schema-change. it is useless
+ TabletSharedPtr tablet = _internal_create_tablet_unlocked(
+ AlterTabletType::SCHEMA_CHANGE, request, is_schema_change,
base_tablet.get(), stores);
if (tablet == nullptr) {
- res = OLAP_ERR_CE_CMD_PARAMS_ERROR;
- LOG(WARNING) << "fail to create tablet. res=" << res;
+ LOG(WARNING) << "fail to create tablet. tablet_id=" <<
request.tablet_id;
+ DorisMetrics::create_tablet_requests_failed.increment(1);
+ return OLAP_ERR_CE_CMD_PARAMS_ERROR;
}
- LOG(INFO) << "finish to process create tablet. res=" << res;
- return res;
-} // create_tablet
+ LOG(INFO) << "success to create tablet. tablet_id=" << tablet_id
+ << ", schema_hash=" << schema_hash;
+ return OLAP_SUCCESS;
+}
TabletSharedPtr TabletManager::_internal_create_tablet_unlocked(
const AlterTabletType alter_type, const TCreateTabletReq& request,
- const bool is_schema_change_tablet, const TabletSharedPtr ref_tablet,
- std::vector<DataDir*> data_dirs) {
- DCHECK((is_schema_change_tablet && ref_tablet != nullptr)
- || (!is_schema_change_tablet && ref_tablet == nullptr));
- // check if the tablet with specified tablet id and schema hash already
exists
- auto checked_tablet = _get_tablet_unlocked(request.tablet_id,
request.tablet_schema.schema_hash);
- if (checked_tablet != nullptr) {
- LOG(WARNING) << "failed to create tablet because tablet already exist."
- << " tablet id = " << request.tablet_id
- << " schema hash = " << request.tablet_schema.schema_hash;
- return nullptr;
- }
- bool is_tablet_added = false;
- auto tablet = _create_tablet_meta_and_dir_unlocked(request,
is_schema_change_tablet, ref_tablet, data_dirs);
+ const bool is_schema_change, const Tablet* base_tablet,
+ const std::vector<DataDir*>& data_dirs) {
+ // If in schema-change state, base_tablet must also be provided.
+ // i.e., is_schema_change and base_tablet are either assigned or not
assigned
+ DCHECK((is_schema_change && base_tablet) || (!is_schema_change &&
!base_tablet));
+
+ // NOTE: The existence of tablet_id and schema_hash has already been
checked,
+ // no need check again here.
+
+ auto tablet = _create_tablet_meta_and_dir_unlocked(request,
is_schema_change,
+ base_tablet, data_dirs);
if (tablet == nullptr) {
return nullptr;
}
+ int64_t new_tablet_id = request.tablet_id;
+ int32_t new_schema_hash = request.tablet_schema.schema_hash;
+
+ // should remove the tablet's pending_id no matter create-tablet success
or not
+ DataDir* data_dir = tablet->data_dir();
+ std::shared_ptr<void> __defer(nullptr, [&](...) {
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
