-
Notifications
You must be signed in to change notification settings - Fork 14.4k
KAFKA-19322: Remove the DelayedOperation constructor that accepts an external lock #19798
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
Conversation
@Mirai1129 please fix the build by removing the usages of the removed constructor |
@chia7712 Thank you so much! Fixed! |
@Mirai1129 please rebase the code to include #19759 |
# Conflicts: # server-common/src/main/java/org/apache/kafka/server/purgatory/DelayedOperation.java
Done! |
public DelayedOperation(long delayMs, Optional<Lock> lockOpt) { | ||
this(delayMs, lockOpt.orElse(new ReentrantLock())); | ||
} | ||
protected final Lock lock = new ReentrantLock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please change the type from Lock
to ReentrantLock
? This class requires some features of ReentrantLock
.
@@ -977,7 +977,7 @@ class ReplicaManager(val config: KafkaConfig, | |||
if (delayedProduceRequestRequired(requiredAcks, entriesPerPartition, initialAppendResults)) { | |||
// create delayed produce operation | |||
val produceMetadata = ProduceMetadata(requiredAcks, initialProduceStatus) | |||
val delayedProduce = new DelayedProduce(timeoutMs, produceMetadata, this, responseCallback, delayedProduceLock) | |||
val delayedProduce = new DelayedProduce(timeoutMs, produceMetadata, this, responseCallback) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove delayedProduceLock
from method signature too
Please cleanup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mirai1129: LGTM
Remove the DelayedOperation constructor that accepts an external lock.
According to this PR.
Reviewers: Ken Huang [email protected], PoAn Yang
[email protected], TengYao Chi [email protected], Chia-Ping Tsai
[email protected]