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

[FLINK-30056][connectors][kafka] Replace deprecated kafka-clients methods #21150

Merged
merged 2 commits into from Nov 25, 2022

Conversation

snuyanzin
Copy link
Contributor

@snuyanzin snuyanzin commented Oct 25, 2022

The PR is replacing deprecated methods
org.apache.kafka.clients.admin.DescribeTopicsResult#all()
org.apache.kafka.clients.consumer.KafkaConsumer#poll(long)
as it was mentioned in corresponding javadocs

What is the purpose of the change

org.apache.kafka.clients.admin.DescribeTopicsResult#all() was made deprecated at https://issues.apache.org/jira/browse/KAFKA-10774
and it is suggested to use org.apache.kafka.clients.admin.DescribeTopicsResult#allTopicNames

org.apache.kafka.clients.consumer.KafkaConsumer#poll(long) was deprecated at https://issues.apache.org/jira/browse/KAFKA-5697
and it's suggested to use org.apache.kafka.clients.consumer.KafkaConsumer#poll(java.time.Duration)

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): ( no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): ( no)
  • The serializers: ( no )
  • The runtime per-record code paths (performance sensitive): ( no )
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: ( no )
  • The S3 file system connector: ( no)

Documentation

  • Does this pull request introduce a new feature? ( no)
  • If yes, how is the feature documented? (not applicable)

@flinkbot
Copy link
Collaborator

flinkbot commented Oct 25, 2022

CI report:

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

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

2 similar comments
@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin snuyanzin changed the title [hotfix] Replace deprecated DescribeTopicsResult#all, KafkaConsumer#poll and ContainerState#getContainerIpAddress [hotfix] Replace deprecated kafka-clients methods Nov 2, 2022
@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

6 similar comments
@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

Copy link
Contributor

@zentol zentol left a comment

Choose a reason for hiding this comment

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

I

Comment on lines -261 to +262
records = consumer.poll(pollTimeout);
records = consumer.poll(Duration.ofMillis(pollTimeout));
Copy link
Contributor

@zentol zentol Nov 15, 2022

Choose a reason for hiding this comment

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

Note: This changes the blocking behavior of this call.

https://cwiki.apache.org/confluence/display/KAFKA/KIP-266%3A+Fix+consumer+indefinite+blocking+behavior

I'd like to see a proper ticket for this particular 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.

Copy link
Contributor Author

@snuyanzin snuyanzin Nov 17, 2022

Choose a reason for hiding this comment

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

please let me know if i need to change commit message or rebase

Copy link
Contributor

Choose a reason for hiding this comment

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

Please split the commits such that 1 is making the poll change, and the other (hotfix) commit with the remaining changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@zentol zentol self-assigned this Nov 15, 2022
@snuyanzin snuyanzin changed the title [hotfix] Replace deprecated kafka-clients methods [FLINK-30056][connectors][kafka] Replace deprecated kafka-clients methods Nov 17, 2022
@snuyanzin snuyanzin force-pushed the kafka_clients branch 2 times, most recently from 22e4694 to 47d13be Compare November 23, 2022 12:19
This changes the blocking behavior, as the previous method essentially ignored the timeout.
@zentol zentol merged commit 5e0e0fd into apache:master Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants