[Bug] information_schema.tables on an external Iceberg catalog loads every table from the remote catalog (unconditional COMMENT fill) #65988

Activity

1. Doris-Breakwater commented on Jul 24, 2026

   

   Doris-Breakwater

   on Jul 24, 2026

   Collaborator

   More actions

   Preliminary maintainer assessment

   Verdict: confirmed code-level performance bug on master, with a high-confidence root cause. The report contains enough evidence to triage without additional logs. Current issue metadata has no labels, assignee, or milestone. A fix is already proposed in #65989, but the current patch has a Thrift contract problem that should be corrected before merge.

   Code-path confirmation

   • BE derives required_columns from the materialized schema-scan slots and sends it in TGetTablesParams (SchemaScanOperatorX -> SchemaTablesScanner::_get_new_table()).
   • FE normalizes that set in FrontendServiceImpl.listTableStatus() and conditionally computes the optional status fields, but currently calls table.getComment() unconditionally.
   • Since [iceberg](fix) Fetch comment PROP from table meta #64263, IcebergExternalTable.getComment() calls properties(), which calls getIcebergTable(). For the reported REST/session-catalog path this reaches Iceberg loadTable, matching the supplied jstack.
   • Therefore, a projection such as TABLE_SCHEMA, TABLE_NAME still performs one metadata load per Iceberg table. The measured latency and request amplification are consistent with this exact call chain.
   • Backward compatibility is already handled by needTableStatusColumn(): if an older BE omits required_columns, its value is null and FE continues to fill the full status.

   Important correction for #65989

   TTableStatus.comment is declared as 3: required string comment in gensrc/thrift/FrontendService.thrift. The current PR simply skips status.setComment(...) when the column is not requested. That leaves the Java field null; generated Thrift Java validation rejects a missing required reference field before writing the response. The C++ scanner also accesses .comment unconditionally because the wire contract says it is required.

   Please keep the required field populated while pruning only the expensive lookup, for example:

   status.setComment(needTableStatusColumn(requiredColumns, "TABLE_COMMENT")
           ? table.getComment()
           : "");

   Initializing the comment to "" and overriding it only when requested is equivalent. Changing the Thrift field to optional is unnecessary and would broaden the compatibility surface.

   Recommended validation

   1. Extend FrontendServiceImplTest so a request without TABLE_COMMENT verifies getComment() is never called, the returned comment is the required empty placeholder, and the result passes Thrift validation/serialization.
   2. Add the positive case: requesting TABLE_COMMENT calls getComment() and returns the real value. Also retain a case with required_columns unset to cover older-BE behavior.
   3. Extend test_tables_status_column_pruning.groovy with a lightweight projection that excludes col=TABLE_COMMENT and a positive projection that includes it. This validates propagation of the required-column set, although the FE mock/serialization test is still needed to catch the current PR defect.
   4. On the reporter's REST catalog, compare the corrected build against the same query and record wall time plus loadTable/table-GET count. The expected result is zero per-table loadTable calls when TABLE_COMMENT is not projected; listing namespaces/tables may still require catalog requests.

   Missing information

   Non-blocking for diagnosis: the exact master commit SHA used for reproduction and the promised before/after timing/request-count result from the patched build. No Doris profile is needed for this FE metadata-RPC issue.

   Suggested next step: label this as a bug affecting Iceberg/external-catalog metadata performance, request the Thrift-safe adjustment and tests on #65989, then merge after the focused FE test and REST-catalog verification pass.

   Breakwater-GitHub-Analysis-Slot: slot_f27888c8b907
2. hello-stephen

   closed this as completedin #65989on Oct 19, 2026