Skip to content

Conversation

@alpreu
Copy link
Contributor

@alpreu alpreu commented Mar 14, 2022

What is the purpose of the change

In 1.15 we ported the Elasticsearch sink to the new unified Sink interface. We initially did not port over the ActionFailureHandler as we wanted to deprecate it. When release-testing the Sink we found out that users depend on having the customizable FailureHandler so we want to re-introduce it to the sink.

Brief change log

  • Added ActionFailureHandler back, adding the failed requests in the ElasticsearchWriter
  • Wrote a unit test that simulates the logic from the Elasticsearch7Example E2E Test.
  • Updated the docs

Verifying this change

This change can be tested with the newly introduced unit test, as well as the old E2E test.

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

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

Documentation

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

@alpreu
Copy link
Contributor Author

alpreu commented Mar 14, 2022

One thing I really don't like is that when we ported the Sink to the unified interface, we created a new RequestIndexer class in the connector.elasticsearch package. The old RequestIndexer remained in the streaming.connectors.elasticsearch package. Now, when reintroducing the ActionFailureHandler we would need to duplicate this interface (as well as the implementations we provide) as well if we wat to deprecate the old one. Using the old one is also an option but then we rely on the old package which we might want to remove/move in the future

@flinkbot
Copy link
Collaborator

flinkbot commented Mar 14, 2022

CI report:

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

@MartijnVisser
Copy link
Contributor

This PR has been closed as the Elasticsearch connector has been moved to https://github.com/apache/flink-connector-elasticsearch - If there's a volunteer who would like to work on this, it's much appreciated :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants