Skip to content

Commit

Permalink
KafkaMessageSource: Lock instead of synchronized
Browse files Browse the repository at this point in the history
Related to #8778

**Cherry-pick to `6.0.x`**
  • Loading branch information
artembilan committed Oct 26, 2023
1 parent 938b270 commit c7b5453
Showing 1 changed file with 103 additions and 43 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018-2022 the original author or authors.
* Copyright 2018-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -30,6 +30,8 @@
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -95,6 +97,7 @@
* @author Mark Norkin
* @author Artem Bilan
* @author Anshul Mehra
* @author Christian Tzolov
*
* @since 5.4
*
Expand All @@ -111,11 +114,13 @@ public class KafkaMessageSource<K, V> extends AbstractMessageSource<Object> impl
*/
public static final String REMAINING_RECORDS = KafkaHeaders.PREFIX + "remainingRecords";

private final Lock lock = new ReentrantLock();

private final ConsumerFactory<K, V> consumerFactory;

private final KafkaAckCallbackFactory<K, V> ackCallbackFactory;

private final Object consumerMonitor = new Object();
private final Lock consumerMonitor = new ReentrantLock();

private final Map<TopicPartition, Set<KafkaAckInfo<K, V>>> inflightRecords = new ConcurrentHashMap<>();

Expand Down Expand Up @@ -385,31 +390,61 @@ private boolean maxPollStringGtr1(Object maxPoll) {
}

@Override
public synchronized boolean isRunning() {
return this.running;
public boolean isRunning() {
this.lock.lock();
try {
return this.running;
}
finally {
this.lock.unlock();
}
}

@Override
public synchronized void start() {
this.running = true;
this.stopped = false;
public void start() {
this.lock.lock();
try {
this.running = true;
this.stopped = false;
}
finally {
this.lock.unlock();
}
}

@Override
public synchronized void stop() {
stopConsumer();
this.running = false;
this.stopped = true;
public void stop() {
this.lock.lock();
try {
stopConsumer();
this.running = false;
this.stopped = true;
}
finally {
this.lock.unlock();
}
}

@Override
public synchronized void pause() {
this.pausing = true;
public void pause() {
this.lock.lock();
try {
this.pausing = true;
}
finally {
this.lock.unlock();
}
}

@Override
public synchronized void resume() {
this.pausing = false;
public void resume() {
this.lock.lock();
try {
this.pausing = false;
}
finally {
this.lock.unlock();
}
}

@Override
Expand All @@ -418,35 +453,43 @@ public boolean isPaused() {
}

@Override // NOSONAR - not so complex
protected synchronized Object doReceive() {
if (this.stopped) {
this.logger.debug("Message source is stopped; no records will be returned");
return null;
}
if (this.consumer == null) {
createConsumer();
this.running = true;
}
if (this.pausing && !this.paused && this.assignedPartitions.size() > 0) {
this.consumer.pause(this.assignedPartitions);
this.paused = true;
}
else if (this.paused && !this.pausing) {
this.consumer.resume(this.assignedPartitions);
this.paused = false;
protected Object doReceive() {
this.lock.lock();
try {

if (this.stopped) {
this.logger.debug("Message source is stopped; no records will be returned");
return null;
}
if (this.consumer == null) {
createConsumer();
this.running = true;
}
if (this.pausing && !this.paused && !this.assignedPartitions.isEmpty()) {
this.consumer.pause(this.assignedPartitions);
this.paused = true;
}
else if (this.paused && !this.pausing) {
this.consumer.resume(this.assignedPartitions);
this.paused = false;
}
if (this.paused && this.recordsIterator == null) {
this.logger.debug("Consumer is paused; no records will be returned");
}
ConsumerRecord<K, V> record = pollRecord();

return record != null
? recordToMessage(record)
: null;
}
if (this.paused && this.recordsIterator == null) {
this.logger.debug("Consumer is paused; no records will be returned");
finally {
this.lock.unlock();
}
ConsumerRecord<K, V> record = pollRecord();

return record != null
? recordToMessage(record)
: null;
}

protected void createConsumer() {
synchronized (this.consumerMonitor) {
this.consumerMonitor.lock();
try {
this.consumer = this.consumerFactory.createConsumer(this.consumerProperties.getGroupId(),
this.consumerProperties.getClientId(), null, this.consumerProperties.getKafkaConsumerProperties());

Expand All @@ -466,6 +509,9 @@ else if (partitions != null) {
rebalanceCallback);
}
}
finally {
this.consumerMonitor.unlock();
}
}

private void assignAndSeekPartitions(TopicPartitionOffset[] partitions) {
Expand Down Expand Up @@ -522,7 +568,8 @@ private ConsumerRecord<K, V> pollRecord() {
return nextRecord();
}
else {
synchronized (this.consumerMonitor) {
this.consumerMonitor.lock();
try {
try {
ConsumerRecords<K, V> records = this.consumer
.poll(this.assignedPartitions.isEmpty() ? this.assignTimeout : this.pollTimeout);
Expand All @@ -545,6 +592,9 @@ private ConsumerRecord<K, V> pollRecord() {
return null;
}
}
finally {
this.consumerMonitor.unlock();
}
}
}

Expand Down Expand Up @@ -590,18 +640,28 @@ private Object recordToMessage(ConsumerRecord<K, V> record) {
}

@Override
public synchronized void destroy() {
stopConsumer();
public void destroy() {
this.lock.lock();
try {
stopConsumer();
}
finally {
this.lock.unlock();
}
}

private void stopConsumer() {
synchronized (this.consumerMonitor) {
this.consumerMonitor.lock();
try {
if (this.consumer != null) {
this.consumer.close(this.closeTimeout);
this.consumer = null;
this.assignedPartitions.clear();
}
}
finally {
this.consumerMonitor.unlock();
}
}

private class IntegrationConsumerRebalanceListener implements ConsumerRebalanceListener {
Expand Down

0 comments on commit c7b5453

Please sign in to comment.