You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "morningman (via GitHub)" <gi...@apache.org> on 2023/03/12 12:47:43 UTC
[GitHub] [doris] morningman commented on a diff in pull request #17590: [Refactor](Metadata tvf) Reconstruct Metadata table-value function into a more general framework.
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: 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