morningman commented on code in PR #18397:
URL: https://github.com/apache/doris/pull/18397#discussion_r1159330559
##########
be/src/olap/rowset/rowset_meta_manager.cpp:
##########
@@ -42,6 +42,17 @@ bool RowsetMetaManager::check_rowset_meta(OlapMeta* meta,
TabletUid tablet_uid,
return meta->key_may_exist(META_COLUMN_FAMILY_INDEX, key, &value);
}
+bool RowsetMetaManager::exists(OlapMeta* meta, TabletUid tablet_uid, const
RowsetId& rowset_id) {
+ std::string key = ROWSET_PREFIX + tablet_uid.to_string() + "_" +
rowset_id.to_string();
+ std::string value;
+ Status s = meta->get(META_COLUMN_FAMILY_INDEX, key, &value);
Review Comment:
Better to return `s` directly instead of a `bool`.
So that if there is other error, we can handle it.
Otherwise, if the `s` is InternalError(for example), this will still return
`true`, which is wrong
##########
be/src/io/fs/file_system.cpp:
##########
@@ -44,6 +46,24 @@ Status FileSystem::delete_file(const Path& file) {
FILESYSTEM_M(delete_file_impl(path));
}
+Status FileSystem::delete_directory_or_file(const Path& path) {
+ auto real_path = absolute_path(path);
Review Comment:
I think we should implement this only in `LocalFileSystem`.
Because you use `stat` to check if it is dir, which is only available for
local fs.
And there is already a method `is_directory()` in local fs, better to use
that.
##########
be/src/olap/tablet.cpp:
##########
@@ -1160,7 +1160,7 @@ bool Tablet::check_rowset_id(const RowsetId& rowset_id) {
return true;
}
}
- if (RowsetMetaManager::check_rowset_meta(_data_dir->get_meta(),
tablet_uid(), rowset_id)) {
+ if (RowsetMetaManager::exists(_data_dir->get_meta(), tablet_uid(),
rowset_id)) {
Review Comment:
Handle the `Status` here
--
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]
