Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HUDI-8378] Fix Avro schema deserializer failing with schema evolution #12111

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vamsikarnika
Copy link
Contributor

Change Logs

Add consumer group id to kafka params based on the schema hash when KafkaAvroSchemaDeserializer is used to avoid kafka connector caching the KafkaConsumer causing issues with schema evolution.

Impact

Kafka connector will create a new Kafka consumer whenever there's a change in the schema instead of using cached kafka consumer.

Risk level (write none, low medium or high below)

NA

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".

  • The config description must be updated if new configs are added or the default value of the configs are changed
  • Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
    ticket number here and follow the instruction to make
    changes to the website.

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@github-actions github-actions bot added the size:S PR with lines of changes in (10, 100] label Oct 16, 2024
@github-actions github-actions bot added size:M PR with lines of changes in (100, 300] and removed size:S PR with lines of changes in (10, 100] labels Dec 10, 2024
}
props.put(KAFKA_VALUE_DESERIALIZER_SCHEMA.key(), schemaProvider.getSourceSchema().toString());
// assign consumer group id based on the schema, since if there's a change in the schema we ensure KafkaRDDIterator doesn't use cached Kafka Consumer
String groupId = props.getString(NATIVE_KAFKA_CONSUMER_GROUP_ID, "");
Copy link
Contributor

@danny0405 danny0405 Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of setting up the group id, why not just fetch the latest schema and set up it again if the schema evolved.

Copy link
Contributor Author

@vamsikarnika vamsikarnika Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The KafkaRDD class from the Kafka client package is caching the KafkaConsumer for a given group id and partition. As a result, we are using KafkaAvroSchemaDeserializer with the previous schema to deserialize the records. This leads to issues during schema evolution.

This changes makes sure that new KafkaConsumer is created when there's a schema change present.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Kafka client itself has an option to control the cache TTL?

Copy link
Contributor Author

@vamsikarnika vamsikarnika Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we copy this utility into hudi and override the flag as false then?

Copy link
Contributor Author

@vamsikarnika vamsikarnika Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if we override the flag to false, we might end up creating new KafkaConsumer every time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we create based on the schema change then? The delta-streamer is a micro-batch for each run, maybe we can add some smart strategy here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will do exactly the same. Caching the KafkaConsumer happens as part of the RDD creation, I'm not sure we can extract that functionality of the kafka utils libr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caching the KafkaConsumer happens as part of the RDD creation

Does it make sense if we hard code the cache as false first then create/reuse the consumer based on the schema change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't control how the KafkaConsumer is created, It's done during the RDD creation. I don't think we can control that.

@vamsikarnika vamsikarnika force-pushed the fix_avro_deserializer branch from 71c4609 to 5680f4c Compare February 1, 2025 15:45
@hudi-bot
Copy link

hudi-bot commented Feb 1, 2025

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:M PR with lines of changes in (100, 300]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants