Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24755 )
Change subject: IMPALA-15144: Fetch and store credentials from REST catalog ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java: http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@910 PS2, Line 910: if (type == ThriftObjectType.DESCRIPTOR_ONLY) { > Wow, your AI gave the same output as my AI gave me before pushing PS3, but Two facts and then it's yours and Zoltan's call. The file is documented as the way to hand metadata to a developer - impala_copy_testcase.xml: "export table or view metadata involved in a query, allowing developers to replay the metadata in a local cluster for testing and debugging purposes". And vended credentials aren't always short-lived: this patch supports static keys with no session token, where getExpiryMs() returns 0 and nothing expires - testToHadoopConfigWithoutSessionTokenKeepsDefaultProvider is that shape. I read the path rather than ran the export against a REST table, so weigh it as such. http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java File fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java: http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@49 PS2, Line 49: public class VendedCredentialsFileIO extends HadoopFileIO > Let's handle REST catalog writing support and this patch separately, I woul Fair enough, scoping it that way is your call. Ordering may go the other way round though: 24735 is in its submit run and 24682 is right behind it, so the adjustment would land on this side. http://gerrit.cloudera.org:8080/#/c/24755/2/fe/src/main/java/org/apache/impala/catalog/iceberg/VendedCredentialsFileIO.java@57 PS2, Line 57: private static final Cache<String, FileSystem> FS_CACHE = > Yes, it's intended, this is the Hadoop-like behavior, and I think the revie One correction to what I wrote: "alive until GC" was wrong. newInstance() goes through Cache.getUnique() -> getInternal(), which puts the instance into the static FileSystem.CACHE map, and only close() takes it out again (hadoop-common 3.3.6 and 3.1.1). So an evicted entry is unreachable from here and still pinned for the process lifetime, S3Client and the two S3AFileSystem pools included - Hadoop's own cache never evicts, so nothing accumulates there. The rate follows loads, not tenants: a LocalCatalog per statement and no cache in IcebergMetaProvider means a REST loadTable per statement, and a rotating session token makes that a new identity and a new instance each time. Still the trade you want? -- To view, visit http://gerrit.cloudera.org:8080/24755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1d9c6e97e17fe8ad6304b49b07fd047cc2a3ffbe Gerrit-Change-Number: 24755 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Rozsa <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Nandor Kollar <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 27 Aug 2026 22:04:42 +0000 Gerrit-HasComments: Yes
