gaborgsomogyi commented on a change in pull request #25760: [SPARK-29054][SS] Invalidate Kafka consumer when new delegation token available URL: https://github.com/apache/spark/pull/25760#discussion_r329771200
########## File path: external/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaDataConsumer.scala ########## @@ -46,6 +47,10 @@ private[kafka010] class InternalKafkaConsumer( val groupId = kafkaParams.get(ConsumerConfig.GROUP_ID_CONFIG).asInstanceOf[String] + // These fields must be updated whenever a new consumer is created. + private[kafka010] var clusterConfig: Option[KafkaTokenClusterConf] = _ Review comment: You understand it well, it contains only config stuff. What I mean here if a delegation token for a specific cluster is not arriving because tokenprovider throws an exception then the actual code won't find any config object. The end result is an empty jaas config on the consumer which will provide a significant crash (I like when things blow up fast and with noise). On the suggested cached version the code would throw `NPE` [here](https://github.com/apache/spark/blob/e1ea806b3075d279b5f08a29fe4c1ad6d3c4191a/external/kafka-0-10-token-provider/src/main/scala/org/apache/spark/kafka010/KafkaTokenUtil.scala#L275). From debug point of view it would be best to communicate through exceptions (but not `NPE` of course) so I think it makes sense to make the change and add a `require` constraint to `getTokenJaasParams`. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
