I am Still not understanding your comments. S3Fileio does not use any Hadoop classes, why should it or any of the others respect a Hadoop configuration. You keep saying users must change something because of a maintenance action but they were configured incorrectly if they assumed a Hadoop config would work with S3Fileio.
On Sat, Nov 7, 2026 at 10:09 AM Sebastian Daberdaku < [email protected]> wrote: > Hi Russell, > > Point 2 of my earlier reply does work for HadoopFileIO and > ResolvingFileIO, agreed. So the shape would be: when table.io() is > HadoopConfigurable the listing uses its configuration; otherwise, it uses > the session configuration as today. > > What it does not fix is the case in #17860 itself. For Glue the default > FileIO is S3FileIO, and S3FileIO, GCSFileIO and ADLSFileIO are not > HadoopConfigurable. A stock Glue catalog with a cross-account role > therefore still lists with the cluster identity after this change and still > gets AccessDenied. The user has three ways out: io-impl set to > ResolvingFileIO, io-impl set to HadoopFileIO, or prefix_listing. The first > keeps S3FileIO underneath and only borrows the Hadoop configuration for the > listing: > > spark.sql.catalog.tenant.io-impl org.apache.iceberg.io.ResolvingFileIO > spark.sql.catalog.tenant.client.factory > org.apache.iceberg.aws.AssumeRoleAwsClientFactory > spark.sql.catalog.tenant.client.assume-role.arn > arn:aws:iam::111122223333:role/lakehouse-access > spark.sql.catalog.tenant.hadoop.fs.s3a.aws.credentials.provider > org.apache.hadoop.fs.s3a.auth.AssumedRoleCredentialProvider > spark.sql.catalog.tenant.hadoop.fs.s3a.assumed.role.arn > arn:aws:iam::111122223333:role/lakehouse-access > spark.sql.catalog.tenant.hadoop.fs.s3a.assumed.role.credentials.provider > com.amazonaws.auth.WebIdentityTokenCredentialsProvider > > It works, but it asks the user to change their FileIO to fix a maintenance > procedure, and to declare the role twice because the listing authenticates > through s3a while everything else goes through the SDK. > > The second option, HadoopFileIO with the same hadoop.* keys, puts listing > and table I/O on one identity path, but every read, write and delete then > goes through s3a instead of S3FileIO: deletes drop from DeleteObjects > batches of up to 1000 keys to one call per file, and the s3.* catalog > properties stop applying. A fallback, not the IO the AWS docs recommend. > > The third option, prefix_listing, is the right path for a non-Hadoop IO, > but today it lists serially on the driver into a single partition (#16932, > #17387), and on Spark Connect that also runs into > spark.rpc.message.maxSize, which a client session cannot raise. So for > S3FileIO users the correct path does not scale and the scalable path has > the wrong credentials. > > Two possible follow-ups, and I would like a steer on which fits: > > 1. Make ResolvingFileIO the default FileIO for the Glue catalog, as it > already is for REST. For s3:// locations it delegates to S3FileIO with the > same properties, so the data path is unchanged, but table.io() becomes > HadoopConfigurable and the stock Glue case is covered by the shape above. > The catch is that ResolvingFileIO needs Hadoop on the classpath, which the > AWS module does not require today, so that is a question for the AWS > maintainers. > > 2. A parallel prefix listing on the executors, so that prefix_listing > gives S3FileIO users the same scalability as the Hadoop walk, with the > table's own credentials and no s3a configuration at all. > > If the HadoopConfigurable shape is what you want for #17862, I will rework > it against the 4.2 module and follow with the 4.1, 4.0 and 3.5 backports. > > Thanks, > Sebastian > > On Sat, Nov 7, 2026 at 1:27 PM Russell Spitzer <[email protected]> > wrote: > >> Sorry I don't understand why number 2 in your first email doesn't work. >> That's what you need to use if you want a Hadoop config. It's the only >> FileIO that understands Hadoop. We have no reason to allow a Hadoop >> configuration for the other IOs. >> >> On Sat, Nov 7, 2026 at 5:48 AM Sebastian Daberdaku < >> [email protected]> wrote: >> >>> Hi Dan and Russell, >>> >>> A proposal to close the API question: replace catalogName(String) with >>> hadoopConf(Configuration), with Dan's FileIO lookup as the default. I agree >>> with Russell that catalogName does not say what it controls. >>> >>> - hadoopConf(Configuration) on DeleteOrphanFilesSparkAction is the >>> Hadoop configuration the listing uses, replacing the session >>> configuration >>> when set. RemoveOrphanFilesProcedure passes the session configuration >>> plus >>> the spark.sql.catalog.<name>.hadoop.* overrides of the catalog it was >>> invoked on, which is what SparkUtil.hadoopConfCatalogOverrides already >>> builds for SparkCatalog. The action keeps wrapping it in >>> SerializableConfiguration and no longer knows about Spark catalog names. >>> - Default, per Dan's suggestion: when hadoopConf is not set and >>> table.io() is HadoopConfigurable, use the configuration it holds; >>> otherwise the session configuration as today. For tables loaded through >>> SparkCatalog that is the session configuration plus the catalog's >>> hadoop.* >>> overrides, so HadoopFileIO and ResolvingFileIO users get the fix without >>> calling anything. This reads the configuration the FileIO was built with, >>> so there is no name-based catalog lookup and no catalog loading side >>> effect, which is what the PR review rejected. It is a change from what I >>> said on the PR, that nothing changes without opting in; I think it >>> is the right default since it is the configuration the table already >>> reads >>> and writes with. Say so if you would rather keep it opt-in. >>> >>> It is still one public method, on the Spark action only, not on the >>> DeleteOrphanFiles interface. The procedure is in a different package, so it >>> has to be public. Going through the generic option(...) map was discussed >>> in the PR review and set aside as the same thing stringly typed, and the >>> action already takes an arbitrary location, so a typed setter for the >>> configuration it lists with adds no new capability. >>> >>> The default alone does not cover #17860: as in my last reply, S3FileIO, >>> GCSFileIO and ADLSFileIO are not HadoopConfigurable, so the listing would >>> still use the session configuration and fail with AccessDenied. The >>> spark.sql.catalog.<name>.hadoop.* overrides are the only per-catalog place >>> a user can configure the Hadoop FileSystem the listing uses, so the >>> procedure passes them. As agreed in the review, the fix stays limited to >>> those explicit overrides; hadoopConfCatalogOverrides keeps its contract and >>> the action does no credential derivation. prefix_listing does authenticate >>> through table.io(), but it lists serially on the driver into a single >>> partition (#16932, #17387), so it is not a drop-in for large tables; making >>> it parallel is a separate change. >>> >>> Does this shape work for you? If so, or if there are no objections by >>> the end of next week (November 15th), I will push it to #17862 this week, >>> retarget the PR to the 4.2 module, ask yangshangqing95 to re-review since >>> his approval was on the catalogName shape, and follow with the 4.1, 4.0 and >>> 3.5 backports. The action tests move from catalogName to hadoopConf, plus >>> one for the default path on a HadoopConfigurable FileIO, and >>> spark-procedures.md gets the default rule. If you would rather see >>> something else, a pointer before I rework it would help. >>> >>> Thanks, >>> Sebastian >>> >>> On Fri, Oct 30, 2026 at 7:34 AM Sebastian Daberdaku < >>> [email protected]> wrote: >>> >>>> Hi Daniel and Russell, >>>> >>>> Thanks for taking another look. I checked whether reading the >>>> configuration off the FileIO would work, and it runs into two problems: >>>> >>>> 1. S3FileIO (as well as GCSFileIO and ADLSFileIO) does not >>>> implement HadoopConfigurable, so there is no Hadoop configuration to >>>> read >>>> from table.io(). The setup in #17860 uses io-impl = S3FileIO, so >>>> for the reported case the check would fall through to the session >>>> configuration and the listing would still fail with AccessDenied. >>>> 2. Where the FileIO is HadoopConfigurable (HadoopFileIO, >>>> ResolvingFileIO), the configuration it holds is the one >>>> SparkCatalog.buildIcebergCatalog passes in, which is already >>>> SparkUtil.hadoopConfCatalogOverrides(spark, name). In those cases the >>>> two >>>> approaches produce the same configuration. >>>> >>>> The spark.sql.catalog.<name>.hadoop.* overrides are the only place a >>>> user can tell the Hadoop listing how to authenticate when the catalog's >>>> FileIO is not Hadoop-based, which is why the PR routes through them. >>>> >>>> Best, >>>> >>>> Sebastian >>>> >>>> On Fri, Oct 30, 2026 at 12:11 AM Russell Spitzer < >>>> [email protected]> wrote: >>>> >>>>> Sorry "bit disconnected" >>>>> >>>>> On Thu, Oct 29, 2026 at 6:10 PM Russell Spitzer < >>>>> [email protected]> wrote: >>>>> >>>>>> I would check out Dan's suggestion. We should try any solutions we >>>>>> can that avoid expanding the API, especially when the connection here >>>>>> between the behavior and the API is a bad disconnected. >>>>>> >>>>>> On Thu, Oct 29, 2026 at 4:53 PM Daniel Weeks <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Sebastian, >>>>>>> >>>>>>> It's been a little while since I looked at this, but wouldn't it >>>>>>> make more sense to interrogate the FileIO to see if it's >>>>>>> HadoopConfigurable >>>>>>> and grab the exact configuration used by the IO rather than go through >>>>>>> this >>>>>>> catalog path? >>>>>>> >>>>>>> At that point, you'd have the exact configuration and overrides for >>>>>>> that specific table. >>>>>>> >>>>>>> -Dan >>>>>>> >>>>>>> On Wed, Oct 28, 2026 at 1:15 AM Sebastian Daberdaku via dev < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Hi all, >>>>>>>> >>>>>>>> I'd like to get some eyes on a PR that fixes a credentials mismatch >>>>>>>> in remove_orphan_files, and agree on the approach before I port it >>>>>>>> across >>>>>>>> Spark versions: >>>>>>>> >>>>>>>> https://github.com/apache/iceberg/pull/17862 (closes #17860) >>>>>>>> >>>>>>>> Problem >>>>>>>> >>>>>>>> Unless prefix_listing is enabled, DeleteOrphanFilesSparkAction >>>>>>>> lists the table location with a Hadoop FileSystem built from the Spark >>>>>>>> session configuration alone, while every other file operation in the >>>>>>>> action >>>>>>>> goes through table.io(). When a catalog reaches its storage with >>>>>>>> credentials other than the cluster's (e.g. a catalog that assumes a >>>>>>>> role in >>>>>>>> another AWS account), reads and deletes succeed but the listing fails >>>>>>>> with >>>>>>>> AccessDenied or silently sees no files. >>>>>>>> >>>>>>>> Change >>>>>>>> >>>>>>>> - DeleteOrphanFilesSparkAction gets a new catalogName(String) >>>>>>>> method. When set, the listing uses the session Hadoop configuration >>>>>>>> plus >>>>>>>> that catalog's spark.sql.catalog.<name>.hadoop.* overrides >>>>>>>> (SparkUtil.hadoopConfCatalogOverrides). When not set, behavior is >>>>>>>> unchanged. >>>>>>>> - RemoveOrphanFilesProcedure passes the catalog it was invoked on, >>>>>>>> so CALL <catalog>.system.remove_orphan_files(...) picks up that >>>>>>>> catalog's >>>>>>>> overrides with no new procedure argument. >>>>>>>> - spark-procedures.md documents which configuration the listing >>>>>>>> uses. >>>>>>>> >>>>>>>> The catalog is passed explicitly rather than inferred from >>>>>>>> Table.name(): as @yangshangqing95 pointed out in review, a table name's >>>>>>>> first segment isn't guaranteed to identify the Spark catalog it came >>>>>>>> from, >>>>>>>> and resolving it via CatalogManager can initialize unrelated catalogs >>>>>>>> as a >>>>>>>> side effect. >>>>>>>> >>>>>>>> Tests cover the procedure path across the test catalogs and the >>>>>>>> action API with both SparkCatalog and SparkSessionCatalog, using a >>>>>>>> marker >>>>>>>> FileSystem registered only in the catalog's hadoop.* overrides. >>>>>>>> >>>>>>>> Questions >>>>>>>> >>>>>>>> 1. Is a catalogName(String) method on the action an acceptable >>>>>>>> public API addition, or would people prefer a different shape? >>>>>>>> 2. The PR was opened against Spark 4.1 before the 4.2 module >>>>>>>> landed. If the direction is agreed, I'll move the change to 4.2 and >>>>>>>> backport it to 4.1, 4.0 and 3.5. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Sebastian Daberdaku >>>>>>>> >>>>>>> >>> *This communication, including any attachments, is intended solely for >>> the recipient(s) addressed. * >>> *If you have received this email in error, please notify the sender >>> immediately and delete it from your system. * >>> *Any unauthorized use, disclosure, or distribution, of this >>> communication is strictly prohibited.* >>> >> > > *This communication, including any attachments, is intended solely for the > recipient(s) addressed. * > *If you have received this email in error, please notify the sender > immediately and delete it from your system. * > *Any unauthorized use, disclosure, or distribution, of this communication > is strictly prohibited.* >
