morningman commented on code in PR #17590:
URL: https://github.com/apache/doris/pull/17590#discussion_r1133244345
##########
gensrc/thrift/FrontendService.thrift:
##########
@@ -717,16 +717,16 @@ struct TInitExternalCtlMetaResult {
enum TSchemaTableName{
Review Comment:
```suggestion
enum TSchemaTableName {
```
##########
be/src/vec/exec/scan/vmeta_scanner.cpp:
##########
@@ -158,38 +153,48 @@ Status VMetaScanner::_fill_block_with_remote_data(const
std::vector<MutableColum
return Status::OK();
}
-Status VMetaScanner::_fetch_iceberg_metadata_batch() {
- VLOG_CRITICAL << "VMetaScanner::_fetch_iceberg_metadata_batch";
+Status VMetaScanner::_fetch_metadata(const TMetaScanRange& meta_scan_range) {
+ VLOG_CRITICAL << "VMetaScanner::_fetch_iceberg_metadata";
+ switch (meta_scan_range.metadata_type) {
+ case TMetadataType::ICEBERG:
+ return _fetch_iceberg_metadata(meta_scan_range);
+ default:
+ _meta_eos = true;
+ break;
+ }
+ return Status::OK();
+}
+
+Status VMetaScanner::_fetch_iceberg_metadata(const TMetaScanRange&
meta_scan_range) {
Review Comment:
the RPC logic is identical for all kinds of metadata fetch request.
I think it should be extracted to a method.
And for different request type, use different method named as
`buildXXXRequest()`
##########
be/src/vec/exec/scan/vmeta_scanner.cpp:
##########
@@ -158,38 +153,48 @@ Status VMetaScanner::_fill_block_with_remote_data(const
std::vector<MutableColum
return Status::OK();
}
-Status VMetaScanner::_fetch_iceberg_metadata_batch() {
- VLOG_CRITICAL << "VMetaScanner::_fetch_iceberg_metadata_batch";
+Status VMetaScanner::_fetch_metadata(const TMetaScanRange& meta_scan_range) {
+ VLOG_CRITICAL << "VMetaScanner::_fetch_iceberg_metadata";
+ switch (meta_scan_range.metadata_type) {
+ case TMetadataType::ICEBERG:
+ return _fetch_iceberg_metadata(meta_scan_range);
+ default:
+ _meta_eos = true;
+ break;
+ }
+ return Status::OK();
+}
+
+Status VMetaScanner::_fetch_iceberg_metadata(const TMetaScanRange&
meta_scan_range) {
+ VLOG_CRITICAL << "VMetaScanner::_fetch_iceberg_metadata";
+ if (!meta_scan_range.__isset.iceberg_params) {
+ return Status::InternalError("Can not find TIcebergMetadataParams from
meta_scan_range.");
+ }
+ // create request
TFetchSchemaTableDataRequest request;
request.cluster_name = "";
request.__isset.cluster_name = true;
- request.schema_table_name = TSchemaTableName::ICEBERG_TABLE_META;
+ request.schema_table_name = TSchemaTableName::METADATA_TABLE;
Review Comment:
use `request.set_xxx()` methods
##########
be/src/vec/exec/scan/vmeta_scanner.cpp:
##########
@@ -158,38 +153,48 @@ Status VMetaScanner::_fill_block_with_remote_data(const
std::vector<MutableColum
return Status::OK();
}
-Status VMetaScanner::_fetch_iceberg_metadata_batch() {
- VLOG_CRITICAL << "VMetaScanner::_fetch_iceberg_metadata_batch";
+Status VMetaScanner::_fetch_metadata(const TMetaScanRange& meta_scan_range) {
+ VLOG_CRITICAL << "VMetaScanner::_fetch_iceberg_metadata";
+ switch (meta_scan_range.metadata_type) {
+ case TMetadataType::ICEBERG:
+ return _fetch_iceberg_metadata(meta_scan_range);
+ default:
+ _meta_eos = true;
+ break;
+ }
+ return Status::OK();
+}
+
+Status VMetaScanner::_fetch_iceberg_metadata(const TMetaScanRange&
meta_scan_range) {
+ VLOG_CRITICAL << "VMetaScanner::_fetch_iceberg_metadata";
+ if (!meta_scan_range.__isset.iceberg_params) {
+ return Status::InternalError("Can not find TIcebergMetadataParams from
meta_scan_range.");
+ }
+ // create request
TFetchSchemaTableDataRequest request;
request.cluster_name = "";
request.__isset.cluster_name = true;
- request.schema_table_name = TSchemaTableName::ICEBERG_TABLE_META;
+ request.schema_table_name = TSchemaTableName::METADATA_TABLE;
request.__isset.schema_table_name = true;
- auto scan_params = _parent->scan_params();
- TMetadataTableRequestParams meta_table_params =
TMetadataTableRequestParams();
- meta_table_params.catalog = scan_params.catalog;
- meta_table_params.__isset.catalog = true;
- meta_table_params.database = scan_params.database;
- meta_table_params.__isset.database = true;
- meta_table_params.table = scan_params.table;
- meta_table_params.__isset.table = true;
-
- meta_table_params.iceberg_metadata_params =
_scan_range.meta_scan_range.iceberg_params;
- meta_table_params.__isset.iceberg_metadata_params = true;
-
- request.metada_table_params = meta_table_params;
+
+ // create TMetadataTableRequestParams
+ TMetadataTableRequestParams metadata_table_params;
+ metadata_table_params.metadata_type = TMetadataType::ICEBERG;
+ metadata_table_params.__isset.metadata_type = true;
+ metadata_table_params.iceberg_metadata_params =
meta_scan_range.iceberg_params;
+ metadata_table_params.__isset.iceberg_metadata_params = true;
+
+ request.metada_table_params = metadata_table_params;
request.__isset.metada_table_params = true;
TNetworkAddress master_addr =
ExecEnv::GetInstance()->master_info()->network_address;
TFetchSchemaTableDataResult result;
-
RETURN_IF_ERROR(ThriftRpcHelper::rpc<FrontendServiceClient>(
master_addr.hostname, master_addr.port,
[&request, &result](FrontendServiceConnection& client) {
client->fetchSchemaTableData(result, request);
},
config::txn_commit_rpc_timeout_ms));
Review Comment:
It is strange to use `txn_commit_rpc_timeout_ms` as the timeout of meta
scanner
--
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]
