adoroszlai commented on a change in pull request #588:
URL: https://github.com/apache/ratis/pull/588#discussion_r790591031
##########
File path:
ratis-server-api/src/main/java/org/apache/ratis/server/leader/LogAppender.java
##########
@@ -137,17 +137,12 @@ default SnapshotInfo shouldInstallSnapshot() {
void run() throws InterruptedException, IOException;
/**
- * Similar to {@link #notify()}, wake up this {@link LogAppender} for an
event, which can be:
+ * Get the {@link AwaitForSignal} for events, which can be:
* (1) new log entries available,
* (2) log indices changed, or
* (3) a snapshot installation completed.
*/
- @SuppressFBWarnings("NN_NAKED_NOTIFY")
- default void notifyLogAppender() {
- synchronized (this) {
- notify();
- }
- }
Review comment:
I think we should keep `notifyLogAppender()` as:
```java
default void notifyLogAppender() {
getEventAwaitForSignal().signal();
}
```
to preserve interface compatibility.
It would also allow keeping existing `notifyLogAppender()` calls and
`LogAppender::notifyLogAppender` references.
##########
File path:
ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -526,15 +519,13 @@ private void installSnapshot(SnapshotInfo snapshot) {
return;
}
- synchronized (this) {
while (isRunning() && !responseHandler.isDone()) {
try {
- wait();
+ getEventAwaitForSignal().await();
} catch (InterruptedException ignored) {
Thread.currentThread().interrupt();
}
Review comment:
Nit: decrease indent by one level.
##########
File path:
ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -571,15 +562,13 @@ private void installSnapshot(TermIndex
firstAvailableLogTermIndex) {
return;
}
- synchronized (this) {
while (isRunning() && !responseHandler.isDone()) {
try {
- wait();
+ getEventAwaitForSignal().await();
} catch (InterruptedException ignored) {
Thread.currentThread().interrupt();
}
}
Review comment:
Nit: decrease indent by one level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
