wangzhigang1999 commented on PR #7451: URL: https://github.com/apache/kyuubi/pull/7451#issuecomment-4945132247
I tested this end-to-end on a real Kyuubi + Spark deployment, comparing the current master behavior with this PR using the same JDBC configurations and `kyuubi-defaults.conf`. The current implementation changes the semantics of unregistered native configurations. For example, with: `hive.metastore.uris=thrift://127.0.0.1:65531` current master passes it to the Spark engine launch command as: `--conf spark.hive.metastore.uris=thrift://127.0.0.1:65531` and the engine fails during initialization as expected because the metastore is unreachable. With this PR, `hive.metastore.uris` is inferred as `HIVE` audience and removed from the Spark launch configuration. The Spark engine therefore initializes successfully using its existing metastore configuration. When the configuration comes from the JDBC session conf, it is forwarded later through `OpenSession`, so `SET hive.metastore.uris` still returns the supplied value. However, that is too late for configurations consumed while initializing Spark, the Hive client, catalog, extensions, or Hadoop subsystems. Seeing the value through `SET` is therefore not equivalent to preserving the previous startup behavior. There is an additional difference for configurations in `kyuubi-defaults.conf`: an unregistered `hive.*` entry is filtered from the Spark launch configuration and is not forwarded later through the session path either. In the tested engine it was simply `<undefined>`. I also tested `spark.*`, `flink.*`, arbitrary custom keys, registered Kyuubi configs, and redaction. `spark.*`, custom keys, registered configs, and redaction behaved as intended. The regression is specifically caused by applying native-prefix audience inference to unregistered configuration keys. I think prefix inference for unregistered native configurations needs to preserve the existing cross-engine/startup behavior. Audience filtering should primarily be applied to registered Kyuubi configuration entries, or unregistered configs should otherwise remain `ANY`, unless there is an explicit and backward-compatible mechanism for selecting their engine audience. -- 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]
