RATIS-2174. Move future.join outside the lock #1168

133tosakarin · 2024-10-15T12:46:21Z

What changes were proposed in this pull request?

In recent observations, I found that some functions wait for a future while holding a lock, which is a very dangerous thing.
To avoid the deadlock problem caused by holding the lock and waiting for the future, I am considering moving the future waiting outside the lock.

Currently, I found that calling changeToFollower in the following functions causes the above situation:

RaftServerImpl.appendEntries
RaftServerImpl.RequestVote
checkAndInstallSnapshot

see https://issues.apache.org/jira/browse/RATIS-2174

133tosakarin · 2024-10-15T14:04:56Z

@szetszwo
please take a look. If you agree to write it this way, I will create an issue in jira.

checkStyle use synchronized(server) instead of synchronized method format exception handle

ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java

ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java

133tosakarin · 2024-10-16T14:41:54Z

@szetszwo @OneSizeFitsQuorum

Please check the comments below. could you help me refer to which one to use as the final solution?
There are currently two main options:

Move future.join outsize the lock (current commit)
Use wait/notify (Code Block below)

Subject: [PATCH] use wait/notify
---
Index: ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderDaemon.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderDaemon.java b/ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderDaemon.java
--- a/ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderDaemon.java	(revision 62ae6d9f068ce01dd467a6fa83fb74dad9c2c8d8)
+++ b/ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderDaemon.java	(date 1729085480985)
@@ -94,6 +94,9 @@
         logAppender.restart();
       }
       closeFuture.complete(finalState);
+      synchronized (logAppender.getServer()) {
+        logAppender.getServer().notifyAll();
+      }
     }
   }
 
Index: ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java	(revision 62ae6d9f068ce01dd467a6fa83fb74dad9c2c8d8)
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/FollowerState.java	(date 1729086757702)
@@ -130,6 +130,9 @@
       runImpl();
     } finally {
       stopped.complete(null);
+      synchronized (server) {
+        server.notifyAll();
+      }
     }
   }
 
Index: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java	(revision 62ae6d9f068ce01dd467a6fa83fb74dad9c2c8d8)
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java	(date 1729087936568)
@@ -565,16 +565,38 @@
     }
   }
 
+   static class Pair<U, V> {
+    private final U first;
+    private final V second;
+
+    private Pair(U first, V second) {
+      this.first = first;
+      this.second = second;
+    }
+
+    static <U,V> Pair<U, V> makePair(U u,  V v) {
+      return new Pair<>(u, v);
+    }
+
+    U first() {
+      return first;
+    }
+
+    V second() {
+      return second;
+    }
+  }
+
   /**
    * Change the server state to Follower if this server is in a different role or force is true.
    * @param newTerm The new term.
    * @param force Force to start a new {@link FollowerState} even if this server is already a follower.
    * @return if the term/votedFor should be updated to the new term
    */
-  private boolean changeToFollower(long newTerm, boolean force, boolean allowListener, Object reason) {
+  private Pair<Boolean, CompletableFuture<Void>> changeToFollower(long newTerm, boolean force, boolean allowListener, Object reason) {
     final AtomicReference<Boolean> metadataUpdated = new AtomicReference<>();
-    changeToFollowerAsync(newTerm, force, allowListener, reason, metadataUpdated).join();
-    return metadataUpdated.get();
+    CompletableFuture<Void> future = changeToFollowerAsync(newTerm, force, allowListener, reason, metadataUpdated);
+    return Pair.makePair(metadataUpdated.get(), future);
   }
 
   private synchronized CompletableFuture<Void> changeToFollowerAsync(
@@ -617,20 +639,37 @@
       long newTerm,
       boolean allowListener,
       Object reason) throws IOException {
-    if (changeToFollower(newTerm, false, allowListener, reason)) {
-      state.persistMetadata();
+    Pair<Boolean, CompletableFuture<Void>> pair = changeToFollower(newTerm, false, allowListener, reason);
+    try {
+      if (pair.first()) {
+        state.persistMetadata();
+      }
+    } finally {
+      waitFutureAndJoin(pair.second());
     }
   }
 
-  synchronized void changeToLeader() {
+  synchronized void waitFutureAndJoin(CompletableFuture<?> future) {
+    while (!future.isDone()) {
+      try {
+        this.wait();
+      } catch (InterruptedException e) {
+        // ignore
+      }
+    }
+    future.join();
+  }
+  
+  synchronized CompletableFuture<Void> changeToLeader() {
     Preconditions.assertTrue(getInfo().isCandidate());
-    role.shutdownLeaderElection();
+    CompletableFuture<Void> future = role.shutdownLeaderElection();
     setRole(RaftPeerRole.LEADER, "changeToLeader");
     final LeaderStateImpl leader = role.updateLeaderState(this);
     state.becomeLeader();
 
     // start sending AppendEntries RPC to followers
     leader.start();
+    return future;
   }
 
   @Override
@@ -1460,8 +1499,9 @@
       final boolean voteGranted = context.decideVote(candidate, candidateLastEntry);
       if (candidate != null && phase == Phase.ELECTION) {
         // change server state in the ELECTION phase
-        final boolean termUpdated =
-            changeToFollower(candidateTerm, true, false, "candidate:" + candidateId);
+        Pair<Boolean, CompletableFuture<Void>> pair = changeToFollower(candidateTerm, true, false, "candidate:" + candidateId);
+        final boolean termUpdated = pair.first;
+        waitFutureAndJoin(pair.second);
         if (voteGranted) {
           state.grantVote(candidate.getId());
         }
Index: ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java	(revision 62ae6d9f068ce01dd467a6fa83fb74dad9c2c8d8)
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java	(date 1729087936578)
@@ -242,6 +242,9 @@
       runImpl();
     } finally {
       stopped.complete(null);
+      synchronized (server) {
+        server.notifyAll();
+      }
     }
   }
 
@@ -256,7 +259,7 @@
       for (int round = 0; shouldRun(); round++) {
         if (skipPreVote || askForVotes(Phase.PRE_VOTE, round)) {
           if (askForVotes(Phase.ELECTION, round)) {
-            server.changeToLeader();
+            server.changeToLeader().join();
           }
         }
       }

szetszwo · 2024-10-16T17:53:51Z

@133tosakarin , sorry for the delay. Will review this soon.

szetszwo

@133tosakarin , thanks a lot for working on this! The idea is great.

We should avoid using Optional. Using final is better since the compiler will enforce that the variable must be initialized before use.
Let's pass an AtomicBoolean instead of adding the new Pair class. It will have much less code change.

See https://issues.apache.org/jira/secure/attachment/13072253/1168_review.patch

133tosakarin · 2024-10-17T06:24:46Z

@133tosakarin , thanks a lot for working on this! The idea is great.

We should avoid using Optional. Using final is better since the compiler will enforce that the variable must be initialized before use.

Let's pass an AtomicBoolean instead of adding the new Pair class. It will have much less code change.

See https://issues.apache.org/jira/secure/attachment/13072253/1168_review.patch

Sze, thank you for your advice!

Our current commit is to move future.join out of the lock, but OneSizeFitsQuorum thinks there may be concurrency issues.

What do you think?

szetszwo · 2024-10-17T15:39:38Z

Suppose we have the following code:

synchronized(server) {
  ...
  final CompletableFuture<Void> future = state.shudown()
  f.join()
}

I agree with you that calling join() at the end of synchronized(server) block seems useless since it just waits for the state thread to terminate. If anything needs to be synchronized, it should be done inside the shutdown() method. Of course, the state thread could be synchronized(server) in its run() method. The same synchronization will happen also in the non-shutdown cases. It there is a bug, the non-shutdown cases should also have the same bug.
I could see that calling join() at the middle of synchronized(server) may be useful in some cases -- the caller wants to wait of the state thread to complete for some reasons such as getting a final result. However, we are not doing that in our shutdown code.

133tosakarin · 2024-10-18T02:42:53Z

Suppose we have the following code:
synchronized(server) {
  ...
  final CompletableFuture<Void> future = state.shudown()
  f.join()
}
I agree with you that calling join() at the end of synchronized(server) block seems useless since it just waits for the state thread to terminate. If anything needs to be synchronized, it should be done inside the shutdown() method. Of course, the state thread could be synchronized(server) in its run() method. The same synchronization will happen also in the non-shutdown cases. It there is a bug, the non-shutdown cases should also have the same bug.

I could see that calling join() at the middle of synchronized(server) may be useful in some cases -- the caller wants to wait of the state thread to complete for some reasons such as getting a final result. However, we are not doing that in our shutdown code.

Thank you for your valuable advice! We have reached a consensus

Regarding the question raised by OneSizeFitsQuorum, the concern might be that when thread A waits for the state to shut down outside the synchronized (server) block, it is possible that the state thread could modify some data of the server. When thread A is woken up, since some server data may have been changed by the state thread before its shutdown, the question is whether this could have any subsequent impact on thread A.

OneSizeFitsQuorum recommends writing it as follows:

For Thread A:

synchronized (server) {
    future = state.shutdown();
    while (!future.isDone()) {
        server.wait();
    }
}

For Thread State:

Future<Void> stopped;
...
try {
    run();
} finally {
    stopped.complete();
    synchronized (server) {
        server.notify();
    }
}

In our code, the State thread may be LogAppender, FollowerState, LeaderElection

szetszwo

@133tosakarin , thanks for the update!

Please don't use CompletableFuture.runAsync(future::join);. We either want to

sync: call future.join(), or
async: do nothing (the code is already async).

See the details inlined.

szetszwo · 2024-10-18T03:00:11Z

ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java

@@ -333,7 +333,7 @@ private boolean askForVotes(Phase phase, int round) throws InterruptedException,
    LOG.info("{} {} round {}: submit vote requests at term {} for {}", this, phase, round, electionTerm, conf);
    final ResultAndTerm r = submitRequestAndWaitResult(phase, conf, electionTerm);
    LOG.info("{} {} round {}: result {}", this, phase, round, r);
-
+    CompletableFuture<Void> future = null;


It is better to use final CompletableFuture<Void> future;. Then future.join() will never have NPE.

Thank you for your valuable advice

szetszwo · 2024-10-18T03:03:09Z

ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java

+        state.persistMetadata();
+      }
+    } catch (IOException e) {
+      CompletableFuture.runAsync(future::join);


CompletableFuture.runAsync means use another thread to call join(). It is not useful. More details:

Without this line, the shutdown thread will run and complete the future.

With this line, the shutdown thread will run and complete the future. In addition, we use another thread to wait for the shutdown thread. Why use one more thread?

Assuming we encounter an exception here, we either do not handle the future or need another thread to handle it. If we do not use another thread to handle the future, there is still a deadlock problem of holding the lock and waiting for the future.

The same consideration applies to using runAsync elsewhere

If we remove line 620, how could we have a deadlock? Could you give more details?

Replacing CompleteFuture.runAsync(future::join) with future.join still presents the deadlock issue where thread A holds the lock while waiting for the future, and thread B needs to acquire the lock.

If we simply ignore the future and throw an exception without handling it, then no deadlock will occur.

Replacing CompleteFuture.runAsync(future::join) with future.join ...

I mean "remove the line", not "replace it with future.join", i.e. case 3 below.

Three cases:

With future.join(), the current thread will wait for the shutdown thread to complete

With CompleteFuture.runAsync(future::join), the current thread and the shutdown thread will run in parallel. In addition, it uses one more thread to wait for the shutdown thread.

With an empty line, the current thread and the shutdown thread will run in parallel.

I see.

case 1. We should avoid this situation, which seems to contradict our issue
case 2. This writing style does not seem to be recommended
case 3. It seems that we can only use this

Then we can use the third case

szetszwo · 2024-10-18T03:19:35Z

Version 1

synchronized (server) {
    future = state.shutdown();
    while (!future.isDone()) {
        server.wait();
    }
}

try {
    run();
} finally {
    stopped.complete();
    synchronized (server) {
        server.notify();
    }
}

Version 2

synchronized (server) {
    future = state.shutdown();
}
future.join()

try {
    run();
} finally {
    stopped.complete();
}

Version 1 and Version 2 seem to be the same. Could you point out when they will be different?

133tosakarin · 2024-10-18T03:32:56Z

Version 1

synchronized (server) {
    future = state.shutdown();
    while (!future.isDone()) {
        server.wait();
    }
}

try {
    run();
} finally {
    stopped.complete();
    synchronized (server) {
        server.notify();
    }
}

Version 2

synchronized (server) {
    future = state.shutdown();
}
future.join()

try {
    run();
} finally {
    stopped.complete();
}

Version 1 and Version 2 seem to be the same. Could you point out when they will be different?

For version 1, we need to wait for another thread to wake up before continuing execution.

For version 2, the current thread and the thread that needs to be shut down may execute in parallel.

szetszwo · 2024-10-18T03:35:44Z

For version 2, the current thread and the thread that needs to be shut down may execute in parallel.

No. future.join() means that it is waiting for stopped to complete.

133tosakarin · 2024-10-18T05:00:14Z

BTW, future.thenApply(..) is not the same as join(). It still returns a un-joined future.

Thank you very much for your explanation.

szetszwo · 2024-10-18T06:26:15Z

ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java

      state.setLeader(leaderId, "installSnapshot");

      server.updateLastRpcTime(FollowerState.UpdateType.INSTALL_SNAPSHOT_START);
      if (snapshotChunkRequest.getRequestIndex() == 0) {
        nextChunkIndex.set(0);
      } else if (nextChunkIndex.get() != snapshotChunkRequest.getRequestIndex()) {
+        // exception new throw, we need join future in another thread
+        CompletableFuture.runAsync(future::join);


@133tosakarin , we need to remove all CompletableFuture.runAsync(future::join). They are not useful. See #1168 (comment)

Ok, I will modify this part of the code.

szetszwo

@133tosakarin , thanks for the update! Please see the comment below and the comments inlined.

In LeaderElection, it should NOT complete stopped in shutdown() since the thread is still running at that time.

+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
@@ -227,7 +227,6 @@ class LeaderElection implements Runnable {
 
   CompletableFuture<Void> shutdown() {
     lifeCycle.checkStateAndClose();
-    stopped.complete(null);
     return stopped;
   }

szetszwo · 2024-10-18T15:37:22Z

ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java

      RaftPeerId leaderId) throws IOException {
    final long currentTerm;
    final long leaderTerm = request.getLeaderTerm();
    final InstallSnapshotRequestProto.SnapshotChunkProto snapshotChunkRequest = request.getSnapshotChunk();
    final TermIndex lastIncluded = TermIndex.valueOf(snapshotChunkRequest.getTermIndex());
    final long lastIncludedIndex = lastIncluded.getIndex();
+    final CompletableFuture<Void> future;
+    final InstallSnapshotReplyProto reply;


Remove reply and declare it in the code below.

ok, thanks for your comment

szetszwo · 2024-10-18T15:38:01Z

ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java

@@ -186,8 +189,9 @@ private InstallSnapshotReplyProto checkAndInstallSnapshot(InstallSnapshotRequest
        // have a lot of requests
        if (state.getLog().getLastCommittedIndex() >= lastIncludedIndex) {
          nextChunkIndex.set(snapshotChunkRequest.getRequestIndex() + 1);
-          return toInstallSnapshotReplyProto(leaderId, getMemberId(),
+          reply =  toInstallSnapshotReplyProto(leaderId, getMemberId(),


final InstallSnapshotReplyProto reply = toInstallSnapshotReplyProto(leaderId, getMemberId(),

szetszwo · 2024-10-18T16:25:54Z

ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java

+    return future.thenApply(dummy -> toInstallSnapshotReplyProto(leaderId, getMemberId(),
+        currentTerm, snapshotChunkRequest.getRequestIndex(), InstallSnapshotResult.SUCCESS));


Create the reply first since it does not have to wait for the shutdown.

final InstallSnapshotReplyProto reply = toInstallSnapshotReplyProto(leaderId, getMemberId(), currentTerm, snapshotChunkRequest.getRequestIndex(), InstallSnapshotResult.SUCCESS); return future.thenApply(dummy -> reply);

szetszwo · 2024-10-18T16:27:18Z

ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java

      InstallSnapshotRequestProto request, RaftPeerId leaderId) throws IOException {
    final long currentTerm;
    final long leaderTerm = request.getLeaderTerm();
    final TermIndex firstAvailableLogTermIndex = TermIndex.valueOf(
        request.getNotification().getFirstAvailableTermIndex());
    final long firstAvailableLogIndex = firstAvailableLogTermIndex.getIndex();
+    final CompletableFuture<Void> future;
+    final InstallSnapshotReplyProto replyProto;


Similar to before, move down the declaration, create the reply and return future.thanApply(..).

szetszwo · 2024-10-18T16:27:55Z

ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java

    }
  }

-  private RoleInfoProto getRoleInfoProto(RaftPeer leader) {
+  private RoleInfoProto getRoleInfoProto (RaftPeer leader){


Please revert this whitespace change.

szetszwo

+1 the change looks good

(Used the web editor to revert a whitespace change but it did not go well. Then, edited it locally and pushed another whitespace reverts.)

…es/runImpl

133tosakarin · 2024-10-19T04:45:12Z

Remove join in FollowerState and LeaderElection.

For FollowerState, changing to Candidate will close FollowerState and then return to FollowerState.stopped, which is equivalent to waiting for oneself to close.

LeaderElection is similar to FollowerState. When changed to Leader, LeaderElection will be closed and then returned to LeaderElection.stop.

133tosakarin · 2024-10-19T07:21:44Z

There seems to be a strange error: the address has already been used

szetszwo · 2024-10-19T19:18:09Z

There seems to be a strange error: the address has already been used

It happens occasionally. It may be a test problem (or library, OS bugs.) Not sure if there is a way to prevent it.

Let's simply rerun the tests for now.

…geToLeader, changeToCandidate

133tosakarin · 2024-10-21T09:38:12Z

#[d0cd3de]

133tosakarin · 2024-10-21T09:50:12Z

In this commit [d0cd3de ] , the following modifications are mainly made

Since Follower and Candidate close themselves, calling future.join at this time will deadlock, so cancel the corresponding return or ignore it.
There are still some tests with LeakSize > 0, I'm not sure if this is related to the current modification.

szetszwo

@133tosakarin , good job! The latest change can pass everything at the first run.

133tosakarin marked this pull request as draft October 15, 2024 12:47

133tosakarin force-pushed the move_future_out_of_the_lock branch 2 times, most recently from 2943809 to 2ec2d11 Compare October 15, 2024 13:44

133tosakarin marked this pull request as ready for review October 15, 2024 14:02

133tosakarin changed the title ~~Demo for moving future.join outside the lock~~ Move future.join outside the lock Oct 15, 2024

133tosakarin changed the title ~~Move future.join outside the lock~~ RATIS-2174. Move future.join outside the lock Oct 16, 2024

format

2b0ce8f

checkStyle use synchronized(server) instead of synchronized method format exception handle

133tosakarin force-pushed the move_future_out_of_the_lock branch from bcc128e to 2b0ce8f Compare October 16, 2024 03:08

checkstyle

d4e072c

OneSizeFitsQuorum reviewed Oct 16, 2024

View reviewed changes

checkstyle

0b5df03

133tosakarin added 4 commits October 17, 2024 10:26

checkstyle

4c6719f

exception handle

60a0c3d

format

139be1e

format

a7d8a9c

szetszwo reviewed Oct 17, 2024

View reviewed changes

133tosakarin added 2 commits October 17, 2024 12:38

remove pair

5a228b4

fix

bb4520d

133tosakarin added 2 commits October 17, 2024 14:25

checkstyle

9123b1e

catch IOException then async execute future.join

309dd5c

szetszwo reviewed Oct 18, 2024

View reviewed changes

133tosakarin added 2 commits October 18, 2024 13:15

use future.thenApply

62e74b6

Add the final keyword to some temporary variables

ea7ff59

szetszwo reviewed Oct 18, 2024

View reviewed changes

133tosakarin added 4 commits October 18, 2024 14:31

remove all runAsync

3f65d6f

fix break is ignored

091bce9

fix break is ignored

3363969

join at close

8eb3dba

szetszwo reviewed Oct 18, 2024

View reviewed changes

133tosakarin and others added 4 commits October 19, 2024 00:39

Change the way variables are defined

756a305

format

d0f00ae

Revert whitespace change.

421583d

Revert whitespace changes.

f8a72fc

szetszwo approved these changes Oct 18, 2024

View reviewed changes

revert join at close, followerState.runImpl, LeaderElection.askForVot…

9a50ce8

…es/runImpl

133tosakarin added 3 commits October 20, 2024 17:44

fix

3d051e0

Merge branch 'master' into move_future_out_of_the_lock

a445d49

add sync for LeaderElection.shouldRun and revert return value in chan…

d0cd3de

…geToLeader, changeToCandidate

133tosakarin added 3 commits October 21, 2024 17:51

format

da1cc16

remove useless line

f9b74f6

revert and fix spotbugs

a7264ef

szetszwo approved these changes Oct 21, 2024

View reviewed changes

szetszwo merged commit d08ba81 into apache:master Oct 21, 2024
12 checks passed

OneSizeFitsQuorum pushed a commit that referenced this pull request Oct 22, 2024

RATIS-2174. Move future.join outside the lock (#1168)

5cd8971

OneSizeFitsQuorum pushed a commit to OneSizeFitsQuorum/ratis that referenced this pull request Nov 1, 2024

RATIS-2174. Move future.join outside the lock (apache#1168)

7f10888

		return future.thenApply(dummy -> toInstallSnapshotReplyProto(leaderId, getMemberId(),
		currentTerm, snapshotChunkRequest.getRequestIndex(), InstallSnapshotResult.SUCCESS));

RATIS-2174. Move future.join outside the lock #1168

RATIS-2174. Move future.join outside the lock #1168

Conversation

133tosakarin commented Oct 15, 2024 • edited Loading

What changes were proposed in this pull request?

133tosakarin commented Oct 15, 2024

133tosakarin commented Oct 16, 2024

szetszwo commented Oct 16, 2024

szetszwo left a comment

Choose a reason for hiding this comment

133tosakarin commented Oct 17, 2024

szetszwo commented Oct 17, 2024

133tosakarin commented Oct 18, 2024

szetszwo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szetszwo commented Oct 18, 2024

133tosakarin commented Oct 18, 2024

szetszwo commented Oct 18, 2024

133tosakarin commented Oct 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szetszwo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szetszwo left a comment

Choose a reason for hiding this comment

133tosakarin commented Oct 19, 2024

133tosakarin commented Oct 19, 2024

szetszwo commented Oct 19, 2024

133tosakarin commented Oct 21, 2024

133tosakarin commented Oct 21, 2024 • edited Loading

szetszwo left a comment

Choose a reason for hiding this comment

133tosakarin commented Oct 15, 2024 •

edited

Loading

szetszwo left a comment •

edited

Loading

133tosakarin commented Oct 21, 2024 •

edited

Loading