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.

Sign up for GitHub

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

Jump to bottom

[pulsar io] make KafkaSourceRecord ack() non-blocking to avoid deadlock #11435

Merged
merged 1 commit into from
Jul 23, 2021
Merged

[pulsar io] make KafkaSourceRecord ack() non-blocking to avoid deadlock #11435

merged 1 commit into from
Jul 23, 2021

Conversation

nlu90
Copy link
Member

@nlu90 nlu90 commented Jul 23, 2021

Motivation

The ack() method of the AbstractKafkaSourceRecord should be non-blocking. Otherwise there'll be deadlock for pulsar-client-io thread and the main public/default/debezium-mongodb-source-0 thread. And further blocks the whole debezium connector to work correctly.

Modifications

  1. remove the blocking future.get() call from ack()
  2. move the commit logic into callbacks

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

For contributor

For this PR, do we need to update docs?

No. Internal bug fix.

@sijie sijie added this to the 2.9.0 milestone Jul 23, 2021
eolivelli
eolivelli approved these changes Jul 23, 2021
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

@eolivelli
Copy link
Contributor

@dlg99 do you mind taking aa look as well ? is this fixing the deadlock you found ?

dlg99
dlg99 approved these changes Jul 23, 2021
View reviewed changes
Copy link
Contributor

@dlg99 dlg99 left a comment

Choose a reason for hiding this comment

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

lgtm.
I didn't run the integration tests (they are not enabled on the CI), I expect @nlu90 did that.

@nlu90
Copy link
Member Author

nlu90 commented Jul 23, 2021

And Here's the integration test result run on my local laptop:

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Apache Pulsar :: Tests 2.9.0-SNAPSHOT:
[INFO]
[INFO] Apache Pulsar :: Tests ............................. SUCCESS [  1.147 s]
[INFO] Apache Pulsar :: Tests :: Integration .............. SUCCESS [06:12 min]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  06:13 min
[INFO] Finished at: 2021-07-23T11:05:54-07:00
[INFO] ------------------------------------------------------------------------
➜  sn-pulsar git:(neng/kafka-source-async-ack) ✗ mvn test -f tests/pom.xml test -DintegrationTestSuiteFile=pulsar-io-sources.xml -DintegrationTests -Dtest=org.apache.pulsar.tests.integration.io.sources.debezium.PulsarDebeziumSourcesTest

@nlu90 nlu90 changed the title make KafkaSourceRecord ack() non-blocking to avoid deadlock [pulsar io] make KafkaSourceRecord ack() non-blocking to avoid deadlock Jul 23, 2021
@sijie sijie merged commit b3892ee into apache:master Jul 23, 2021
@sijie sijie deleted the neng/kafka-source-async-ack branch July 23, 2021 20:13
@nlu90 nlu90 mentioned this pull request Jul 23, 2021
Merged
1 task
codelipenghui pushed a commit that referenced this pull request Jul 30, 2021
@nlu90 @codelipenghui
make KafkaSourceRecord ack() async to avoid deadlock (#11435)
9767a82 
The `ack()` method of the `AbstractKafkaSourceRecord` should be non-blocking. Otherwise there'll be deadlock for pulsar-client-io thread and the main `public/default/debezium-mongodb-source-0` thread. And further blocks the whole debezium connector to work correctly.

1. remove the blocking `future.get()` call from `ack()`
2. move the commit logic into callbacks

(cherry picked from commit b3892ee)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jul 30, 2021
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request Sep 24, 2021
@nlu90 @dlg99
make KafkaSourceRecord ack() async to avoid deadlock (apache#11435)
934a15e 
### Motivation
The `ack()` method of the `AbstractKafkaSourceRecord` should be non-blocking. Otherwise there'll be deadlock for pulsar-client-io thread and the main `public/default/debezium-mongodb-source-0` thread. And further blocks the whole debezium connector to work correctly.


### Modifications

1. remove the blocking `future.get()` call from `ack()`
2. move the commit logic into callbacks
eolivelli pushed a commit to datastax/pulsar that referenced this pull request Sep 24, 2021
@nlu90 @eolivelli
make KafkaSourceRecord ack() async to avoid deadlock (apache#11435)
b3ddb47 
### Motivation
The `ack()` method of the `AbstractKafkaSourceRecord` should be non-blocking. Otherwise there'll be deadlock for pulsar-client-io thread and the main `public/default/debezium-mongodb-source-0` thread. And further blocks the whole debezium connector to work correctly.


### Modifications

1. remove the blocking `future.get()` call from `ack()`
2. move the commit logic into callbacks
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request Jan 11, 2022
@nlu90 @dlg99
make KafkaSourceRecord ack() async to avoid deadlock (apache#11435)
c3bf160 
The `ack()` method of the `AbstractKafkaSourceRecord` should be non-blocking. Otherwise there'll be deadlock for pulsar-client-io thread and the main `public/default/debezium-mongodb-source-0` thread. And further blocks the whole debezium connector to work correctly.

1. remove the blocking `future.get()` call from `ack()`
2. move the commit logic into callbacks
@dlg99 dlg99 mentioned this pull request Jan 11, 2022
Merged
4 tasks
eolivelli pushed a commit to datastax/pulsar that referenced this pull request Jan 11, 2022
@dlg99 @nlu90
make KafkaSourceRecord ack() async to avoid deadlock (apache#11435) (#18
041ce94 
)

The `ack()` method of the `AbstractKafkaSourceRecord` should be non-blocking. Otherwise there'll be deadlock for pulsar-client-io thread and the main `public/default/debezium-mongodb-source-0` thread. And further blocks the whole debezium connector to work correctly.

1. remove the blocking `future.get()` call from `ack()`
2. move the commit logic into callbacks

Co-authored-by: Neng Lu <[email protected]>
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
@nlu90 @ciaocloud
make KafkaSourceRecord ack() async to avoid deadlock (apache#11435)
00b1d45 
### Motivation
The `ack()` method of the `AbstractKafkaSourceRecord` should be non-blocking. Otherwise there'll be deadlock for pulsar-client-io thread and the main `public/default/debezium-mongodb-source-0` thread. And further blocks the whole debezium connector to work correctly.


### Modifications

1. remove the blocking `future.get()` call from `ack()`
2. move the commit logic into callbacks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui codelipenghui approved these changes

@eolivelli eolivelli eolivelli approved these changes

@dlg99 dlg99 dlg99 approved these changes

@sijie sijie sijie approved these changes

Assignees

@nlu90 nlu90

Labels
area/connector cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.1
Projects
None yet
Milestone
2.9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@nlu90 @eolivelli @sijie @dlg99 @codelipenghui