Fix NPE issue during recovery add #3620

zhaohaidao · 2022-11-07T10:13:45Z

Descriptions of the changes in this PR:
Fix NPE issue during recovery add

Motivation

The issue details can is #3618

Changes

Check if the op has been recycled after handling bookie failure

Master Issue: #3618

In order to uphold a high standard for quality for code contributions, Apache BookKeeper runs various precommit
checks for pull requests. A pull request can only be merged when it passes precommit checks.

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

If this PR is a BookKeeper Proposal (BP):

Make sure the PR title is formatted like:
<BP-#>: Description of bookkeeper proposal
e.g. BP-1: 64 bits ledger is support

Attach the master issue link in the description of this PR.

Attach the google doc link if the BP is written in Google Doc.

Otherwise:

Make sure the PR title is formatted like:
<Issue #>: Description of pull request
e.g. Issue 123: Description ...

Make sure tests pass via mvn clean apache-rat:check install spotbugs:check.

Replace <Issue #> in the title with the actual Issue number.

dlg99 · 2022-11-07T21:09:14Z

bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java

@@ -266,6 +266,10 @@ public void run() {
        // We are about to send. Check if we need to make an ensemble change
        // because of delayed write errors
        lh.maybeHandleDelayedWriteBookieFailure();
+        // This op maybe recycled during bookie failures
+        if (maybeRecycled()) {


So maybeHandleDelayedWriteBookieFailure triggers something that recycles this pending add op (I haven't looked that closely)?

Then the fix masks the problem.
One side of the problem is that lh becomes null / results in NPE later.
Another side of the problem is that pending add opp can be recycled and immediately reused by another add.

That's right. maybeHandleDelayedWriteBookieFailure will replace failed bookie with healthy one. However if NotEnoughBookieException is throwed. maybeHandleDelayedWriteBookieFailure will recycle this pending add op.
You can see this recycle logic in followed code logic
`

@Override void handleBookieFailure(final Map<Integer, BookieId> failedBookies) { // handleBookieFailure should always run in the ordered executor thread for this // ledger, so this synchronized should be unnecessary, but putting it here now // just in case (can be removed when we validate threads) synchronized (metadataLock) { ... try { List<BookieId> newEnsemble = EnsembleUtils.replaceBookiesInEnsemble( clientCtx.getBookieWatcher(), metadata, currentEnsemble, failedBookies, logContext); ... } catch (BKException.BKNotEnoughBookiesException e) { LOG.error("Could not get additional bookie to remake ensemble, closing ledger: {}", ledgerId); handleUnrecoverableErrorDuringAdd(e.getCode()); return; }

`

So maybeHandleDelayedWriteBookieFailure triggers something that recycles this pending add op (I haven't looked that closely)?

Then the fix masks the problem. One side of the problem is that lh becomes null / results in NPE later. Another side of the problem is that pending add opp can be recycled and immediately reused by another add.

One side of the problem is that lh becomes null / results in NPE later:
If I understand correctly, only maybeHandleDelayedWriteBookieFailure has a chance to recycle this operation causing lh to become null which leads to this npe issue. Although this behavior is not as described in the followed comments about recycle condition, I think it is reasonable.

private void maybeRecycle() { /** * We have opportunity to recycle two objects here. * PendingAddOp#toSend and LedgerHandle#pendingAddOp * * A. LedgerHandle#pendingAddOp: This can be released after all 3 conditions are met. * - After the callback is run * - After safeRun finished by the executor * - Write responses are returned from all bookies * as BookieClient Holds a reference from the point the addEntry requests are sent. * * B. ByteBuf can be released after 2 of the conditions are met. * - After the callback is run as we will not retry the write after callback * - After safeRun finished by the executor * BookieClient takes and releases on this buffer immediately after sending the data. * * The object can only be recycled after the above conditions are met * otherwise we could end up recycling twice and all * joy that goes along with that. */ if (hasRun && callbackTriggered) { ReferenceCountUtil.release(toSend); toSend = null; } // only recycle a pending add op after it has been run. if (hasRun && toSend == null && pendingWriteRequests == 0) { recyclePendAddOpObject(); } }

Another side of the problem is that pending add opp can be recycled and immediately reused by another add.: I don't understand this very well, can you explain in more detail?

Another side of the problem is that pending add opp can be recycled and immediately reused by another add.

I don't understand this very well, can you explain in more detail?

lh.maybeHandleDelayedWriteBookieFailure(); // Add Op is recycled at this point, maybeRecycled() has a chance to check lh == null // but the recycler can give that op to another thread/request (via RECYCLER.get() call) // so it can be re-initialized with different lh and other data // so maybeRecycled returns false and the rest of the method is executed against different op // to avoid such problems and avoid locks we use ordered executors / guarantee order of calls if (maybeRecycled()) {

@zhaohaidao I hope this helps

Another side of the problem is that pending add opp can be recycled and immediately reused by another add.

I don't understand this very well, can you explain in more detail?

lh.maybeHandleDelayedWriteBookieFailure(); // Add Op is recycled at this point, maybeRecycled() has a chance to check lh == null // but the recycler can give that op to another thread/request (via RECYCLER.get() call) // so it can be re-initialized with different lh and other data // so maybeRecycled returns false and the rest of the method is executed against different op // to avoid such problems and avoid locks we use ordered executors / guarantee order of calls if (maybeRecycled()) {

@zhaohaidao I hope this helps

Thanks a lot for your explanation. I submitted a revision based on your suggestions.
Please have a look if you have time.
@dlg99

hangc0276

@zhaohaidao Thanks for your contribution.

The root cause of this NPE is the different implementations of handleBookieFailure in LedgerHandle and ReadOnlyLedgerHandle.

In the LedgerHandle implementation, it handles ensemble change results in a new task with the same thread

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java

Lines 1962 to 2004 in 43bdfb3

    
           .run().whenCompleteAsync((metadata, ex) -> { 
        
                   if (ex != null) { 
        
                       LOG.warn("{}[attempt:{}] Exception changing ensemble", logContext, attempts.get(), ex); 
        
                       handleUnrecoverableErrorDuringAdd(BKException.getExceptionCode(ex, WriteException)); 
        
                   } else if (metadata.getValue().isClosed()) { 
        
                       if (LOG.isDebugEnabled()) { 
        
                           LOG.debug("{}[attempt:{}] Metadata closed during attempt to replace bookie." 
        
                                     + " Another client must have recovered the ledger.", logContext, attempts.get()); 
        
                       } 
        
                       handleUnrecoverableErrorDuringAdd(BKException.Code.LedgerClosedException); 
        
                   } else if (metadata.getValue().getState() == LedgerMetadata.State.IN_RECOVERY) { 
        
                       if (LOG.isDebugEnabled()) { 
        
                           LOG.debug("{}[attempt:{}] Metadata marked as in-recovery during attempt to replace bookie." 
        
                                     + " Another client must be recovering the ledger.", logContext, attempts.get()); 
        
                       } 
        
                       handleUnrecoverableErrorDuringAdd(BKException.Code.LedgerFencedException); 
        
                   } else { 
        
                       if (LOG.isDebugEnabled()) { 
        
                           LOG.debug("{}[attempt:{}] Success updating metadata.", logContext, attempts.get()); 
        
                       } 
        
                       List<BookieId> newEnsemble = null; 
        
                       Set<Integer> replaced = null; 
        
                       synchronized (metadataLock) { 
        
                           if (!delayedWriteFailedBookies.isEmpty()) { 
        
                               Map<Integer, BookieId> toReplace = new HashMap<>(delayedWriteFailedBookies); 
        
                               delayedWriteFailedBookies.clear(); 
        
                               ensembleChangeLoop(origEnsemble, toReplace); 
        
                           } else { 
        
                               newEnsemble = getCurrentEnsemble(); 
        
                               replaced = EnsembleUtils.diffEnsemble(origEnsemble, newEnsemble); 
        
                               LOG.info("New Ensemble: {} for ledger: {}", newEnsemble, ledgerId); 
        
                               changingEnsemble = false; 
        
                           } 
        
                       } 
        
                       if (newEnsemble != null) { // unsetSuccess outside of lock 
        
                           unsetSuccessAndSendWriteRequest(newEnsemble, replaced); 
        
                       } 
        
                   } 
        
           }, clientCtx.getMainWorkerPool().chooseThread(ledgerId));

In the ReadOnlyLedgerHandle, it handles the ensemble change results and exceptions in the current task and current thread, which will lead to the NPE.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadOnlyLedgerHandle.java

Line 232 in 43bdfb3

handleUnrecoverableErrorDuringAdd(e.getCode());

I suggest keeping the same with LedgerHandle implementation to change handleUnrecoverableErrorDuringAdd(e.getCode()); to executeOrdered(() -> handleUnrecoverableErrorDuringAdd(e.getCode()));

zhaohaidao · 2022-11-08T04:53:58Z

Thanks for your advice. I don't quite understand why processing in a different task and current thread would not be a problem. Could you please explain this in more detail?
@hangc0276

zhaohaidao · 2022-11-13T08:16:38Z

@zhaohaidao Thanks for your contribution.

The root cause of this NPE is the different implementations of handleBookieFailure in LedgerHandle and ReadOnlyLedgerHandle.

In the LedgerHandle implementation, it handles ensemble change results in a new task with the same thread

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java

Lines 1962 to 2004 in 43bdfb3

.run().whenCompleteAsync((metadata, ex) -> {

if (ex != null) {

LOG.warn("{}[attempt:{}] Exception changing ensemble", logContext, attempts.get(), ex);

handleUnrecoverableErrorDuringAdd(BKException.getExceptionCode(ex, WriteException));

} else if (metadata.getValue().isClosed()) {

if (LOG.isDebugEnabled()) {

LOG.debug("{}[attempt:{}] Metadata closed during attempt to replace bookie."

+ " Another client must have recovered the ledger.", logContext, attempts.get());

}

handleUnrecoverableErrorDuringAdd(BKException.Code.LedgerClosedException);

} else if (metadata.getValue().getState() == LedgerMetadata.State.IN_RECOVERY) {

if (LOG.isDebugEnabled()) {

LOG.debug("{}[attempt:{}] Metadata marked as in-recovery during attempt to replace bookie."

+ " Another client must be recovering the ledger.", logContext, attempts.get());

}

handleUnrecoverableErrorDuringAdd(BKException.Code.LedgerFencedException);

} else {

if (LOG.isDebugEnabled()) {

LOG.debug("{}[attempt:{}] Success updating metadata.", logContext, attempts.get());

}

List<BookieId> newEnsemble = null;

Set<Integer> replaced = null;

synchronized (metadataLock) {

if (!delayedWriteFailedBookies.isEmpty()) {

Map<Integer, BookieId> toReplace = new HashMap<>(delayedWriteFailedBookies);

delayedWriteFailedBookies.clear();

ensembleChangeLoop(origEnsemble, toReplace);

} else {

newEnsemble = getCurrentEnsemble();

replaced = EnsembleUtils.diffEnsemble(origEnsemble, newEnsemble);

LOG.info("New Ensemble: {} for ledger: {}", newEnsemble, ledgerId);

changingEnsemble = false;

}

}

if (newEnsemble != null) { // unsetSuccess outside of lock

unsetSuccessAndSendWriteRequest(newEnsemble, replaced);

}

}

}, clientCtx.getMainWorkerPool().chooseThread(ledgerId));

In the ReadOnlyLedgerHandle, it handles the ensemble change results and exceptions in the current task and current thread, which will lead to the NPE.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadOnlyLedgerHandle.java

Line 232 in 43bdfb3

handleUnrecoverableErrorDuringAdd(e.getCode());

I suggest keeping the same with LedgerHandle implementation to change handleUnrecoverableErrorDuringAdd(e.getCode()); to executeOrdered(() -> handleUnrecoverableErrorDuringAdd(e.getCode()));

Thanks a lot for your explanation. I submitted a revision based on your suggestions.
Please have a look if you have time.
@hangc0276

hangc0276 · 2023-01-11T07:32:13Z

@merlimat @eolivelli @dlg99 @zymap @horizonzy Please help take a look at this Pr, thanks a lot.

bookkeeper-server/src/test/java/org/apache/bookkeeper/client/PendingAddOpTest.java

horizonzy · 2023-01-15T09:55:49Z

bookkeeper-server/src/test/java/org/apache/bookkeeper/client/PendingAddOpTest.java

+                        Lists.newArrayList(b1, b2, b3)));
+        ReadOnlyLedgerHandle lh = new ReadOnlyLedgerHandle(clientCtx, 0, md, BookKeeper.DigestType.CRC32C,
+                ClientUtil.PASSWD, true);
+        lh.notifyWriteFailed(0, b1);


In the normal case, it can't happen.
lh.notifyWriteFailed(0, b1); should not before op.run(), it just for test I guess.
I'm curious about why the problem happened. Could you add a test to mock the real case? thanks a lot.

I try to give a scenario where the problem might occur.
The client tries to write 945 and 946 to the same ledger (id=1256) successively, and writes 3 copies of each entry. Write 945 is successful but one of the bookies returns an exception, so LedgerHandle.delayedWriteFailedBookies will not be empty. Then when writing 946, if a BKNotEnoughBookiesException is thrown, an NPE issue will occur.

It is a little troublesome to construct the above scenario, so I ensure that delayedWriteFailedBookies is not empty (by lh.notifyWriteFailed(0, b1)) from the beginning of my test case, so that I can quickly verify the npe issue.

It should be noted that this is a scenario I deduced from the error log. In the log, only 946 write failures caused by BKNotEnoughBookiesException can be seen.

zhaohaidao · 2023-01-20T07:00:45Z

Sorry, this change may introduce a new problem. There is a case: The ledger handles asyncAddEntry 10000 times. So there are 10000 PendingAddOp will be sent to the LedgerHandle#pendingAddOps.

The 10000 PendingAddOp will be executed one by one in the future. Before this change, errorOutPendingAdds is a sync operation; it takes all the PendingAddOp from LedgerHandle#pendingAddOps, then submits a callback. After submitting callback, all the PendingAddOp#callbackTriggered will be set to true. Because we execute every PendingAddOp in the thread pool, every PendingAddOp will be run at some time.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java

Lines 248 to 254 in 7b5b6b2

if (callbackTriggered) {

// this should only be true if the request was failed due

// to another request ahead in the pending queue,

// so we can just ignore this request

maybeRecycle();

return;

}

At now, we check PendingAddOp#callbackTriggered value is true or not; if true, it will return directly, didn't send any request to the server; it's quick.
But if we make errorOutPendingAdds async, all the 10000 PendingAddOp will send requests to the server, because the Runnable is ordered. After 10000 PendingAddOp executing, the errorOutPendingAdds will be executed. It's not acceptable.

@horizonzy Thank you very much for your careful explanation, your concern is very reasonable. However in the scenario you described, when bkclient writes multiple messages to the same ledger. If the first message fails, LedgerHandle makes subsequent requests fail by calling errorOutPendingAdds asynchronously.
There is a difference between the implementation of ReadOnlyLedgeHandle and the implementation of LedgerHandle. The latter is a sync operation.
The change of this pr is actually referring to the implementation of handleUnrecoverableErrorDuringAdd of LedgerHandle, according to the suggestion of @hangc0276 .
Another fix I can think of is to interrupt the execution of safeRun when NotEnoughException is encountered, but this may require adding an exception signature to handleBookieFailure. I understand that this may violate the original design intention? Do you have any better suggestions for changes? Welcome to continue the discussion.

It is worth mentioning that although LedgerHandle.handleUnrecoverableErrorDuringAdd may also call errorOutPendingAdds synchronously, the logic of this code will never happen, because the ledger recovery process will definitely call the ReadOnlyLedgeHandle method.

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java

Lines 1767 to 1777 in 7b5b6b2

    
           // close the ledger and send fails to all the adds in the pipeline 
        
           void handleUnrecoverableErrorDuringAdd(int rc) { 
        
               if (getLedgerMetadata().getState() == LedgerMetadata.State.IN_RECOVERY) { 
        
                   // we should not close ledger if ledger is recovery mode 
        
                   // otherwise we may lose entry. 
        
                   errorOutPendingAdds(rc); 
        
                   return; 
        
               } 
        
               LOG.error("Closing ledger {} due to {}", ledgerId, BKException.codeLogger(rc)); 
        
               asyncCloseInternal(NoopCloseCallback.instance, null, rc); 
        
           }

horizonzy · 2023-01-20T07:48:26Z

There are some places to invoke handleUnrecoverableErrorDuringAdd, the LedgerHandle will also invoke handleUnrecoverableErrorDuringAdd. This pr also change the behavior in LedgerHandle.

zhaohaidao · 2023-01-20T09:27:50Z

bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java

@@ -1769,7 +1769,7 @@ void handleUnrecoverableErrorDuringAdd(int rc) {
        if (getLedgerMetadata().getState() == LedgerMetadata.State.IN_RECOVERY) {
            // we should not close ledger if ledger is recovery mode
            // otherwise we may lose entry.
-            errorOutPendingAdds(rc);
+            executeOrdered(() -> errorOutPendingAdds(rc));


@horizonzy I may not have explained it clearly. Although this is changed to an async operation, this code block will never be executed, because when in IN_RECOVERY mode, the method of ReadOnlyLedgerHandle must be called.
What do you think if I roll back the changes to LedgerHandle and only keep the changes to ReadOnlyLedgerHandle

zhaohaidao · 2023-01-20T11:07:32Z

rerun failure checks

zhaohaidao · 2023-01-20T13:46:44Z

rerun failure checks

zhaohaidao · 2023-01-20T14:55:55Z

rerun failure checks

zhaohaidao · 2023-01-28T09:16:07Z

rerun failure checks

dlg99

LGTM.
Thank you for fixing this and adding the test.

zhaohaidao · 2023-09-16T03:25:47Z

Hey, @hangc0276 Can you help me continue to review this pr when you are free? Also, the ci seems to be stuck and cannot be re-triggered.

zhaohaidao · 2023-09-16T05:40:27Z

rerun failure checks

…e-issue-during-recovery-add

zhaohaidao · 2023-10-09T14:13:50Z

rerun failure checks

zhaohaidao · 2023-10-11T12:52:28Z

rerun failure checks

zhaohaidao · 2023-10-11T18:51:23Z

rerun failure checks

zhaohaidao · 2023-10-23T14:34:59Z

rerun failure checks

zhaohaidao added 2 commits November 7, 2022 18:11

Fix NPE issue during recovery add

ffa2458

Rollback useless change

636d8bd

hangc0276 requested review from eolivelli, Vanlightly, hangc0276, dlg99, merlimat, hezhangjian, StevenLuMT and zymap November 7, 2022 10:37

hangc0276 assigned zhaohaidao Nov 7, 2022

hangc0276 added the type/bug label Nov 7, 2022

hangc0276 added this to the 4.16.0 milestone Nov 7, 2022

zhaohaidao added 2 commits November 7, 2022 18:40

Fix licence warnings

96a2f82

Fix checkstyle warnings

d554c64

dlg99 reviewed Nov 7, 2022

View reviewed changes

hangc0276 requested changes Nov 8, 2022

View reviewed changes

hangc0276 added release/4.14.6 release/4.15.4 labels Nov 8, 2022

Address comments

1836930

hangc0276 added release/4.14.7 and removed release/4.14.6 labels Nov 14, 2022

horizonzy reviewed Jan 15, 2023

View reviewed changes

bookkeeper-server/src/test/java/org/apache/bookkeeper/client/PendingAddOpTest.java Outdated Show resolved Hide resolved

horizonzy reviewed Jan 15, 2023

View reviewed changes

Address comments

1cf0ba7

zhaohaidao removed request for merlimat and Vanlightly January 19, 2023 15:20

zhaohaidao commented Jan 20, 2023

View reviewed changes

Rollback changes to LedgerHandle

7848138

hangc0276 added release/4.14.8 and removed release/4.14.7 labels Feb 9, 2023

zymap added release/4.15.5 and removed release/4.15.4 labels Feb 21, 2023

hangc0276 modified the milestones: 4.16.0, 4.17.0 Mar 13, 2023

dlg99 approved these changes Apr 21, 2023

View reviewed changes

hangc0276 added release/4.14.9 and removed release/4.14.8 labels Jun 25, 2023

hangc0276 added release/4.15.6 and removed release/4.15.5 labels Sep 11, 2023

hangc0276 closed this Oct 7, 2023

hangc0276 reopened this Oct 7, 2023

Merge remote-tracking branch 'offical-upstream/master' into hotfix/np…

69a4879

…e-issue-during-recovery-add

Fix compile err

ddc33e7

eolivelli removed this from the 4.17.0 milestone May 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix NPE issue during recovery add #3620

Fix NPE issue during recovery add #3620

zhaohaidao commented Nov 7, 2022

dlg99 Nov 7, 2022

zhaohaidao Nov 8, 2022

zhaohaidao Nov 8, 2022

dlg99 Nov 8, 2022

zhaohaidao Nov 13, 2022

hangc0276 left a comment

zhaohaidao commented Nov 8, 2022 •

edited

Loading

zhaohaidao commented Nov 13, 2022

hangc0276 commented Jan 11, 2023

horizonzy Jan 15, 2023

zhaohaidao Jan 19, 2023

zhaohaidao commented Jan 20, 2023

horizonzy commented Jan 20, 2023

zhaohaidao Jan 20, 2023

horizonzy Jan 20, 2023

zhaohaidao commented Jan 20, 2023

zhaohaidao commented Jan 20, 2023

zhaohaidao commented Jan 20, 2023

zhaohaidao commented Jan 28, 2023

dlg99 left a comment

zhaohaidao commented Sep 16, 2023

zhaohaidao commented Sep 16, 2023

zhaohaidao commented Oct 9, 2023

zhaohaidao commented Oct 11, 2023

zhaohaidao commented Oct 11, 2023

zhaohaidao commented Oct 23, 2023

	.run().whenCompleteAsync((metadata, ex) -> {
	if (ex != null) {
	LOG.warn("{}[attempt:{}] Exception changing ensemble", logContext, attempts.get(), ex);
	handleUnrecoverableErrorDuringAdd(BKException.getExceptionCode(ex, WriteException));
	} else if (metadata.getValue().isClosed()) {
	if (LOG.isDebugEnabled()) {
	LOG.debug("{}[attempt:{}] Metadata closed during attempt to replace bookie."
	+ " Another client must have recovered the ledger.", logContext, attempts.get());
	}
	handleUnrecoverableErrorDuringAdd(BKException.Code.LedgerClosedException);
	} else if (metadata.getValue().getState() == LedgerMetadata.State.IN_RECOVERY) {
	if (LOG.isDebugEnabled()) {
	LOG.debug("{}[attempt:{}] Metadata marked as in-recovery during attempt to replace bookie."
	+ " Another client must be recovering the ledger.", logContext, attempts.get());
	}

	handleUnrecoverableErrorDuringAdd(BKException.Code.LedgerFencedException);
	} else {
	if (LOG.isDebugEnabled()) {
	LOG.debug("{}[attempt:{}] Success updating metadata.", logContext, attempts.get());
	}

	List<BookieId> newEnsemble = null;
	Set<Integer> replaced = null;
	synchronized (metadataLock) {
	if (!delayedWriteFailedBookies.isEmpty()) {
	Map<Integer, BookieId> toReplace = new HashMap<>(delayedWriteFailedBookies);
	delayedWriteFailedBookies.clear();

	ensembleChangeLoop(origEnsemble, toReplace);
	} else {
	newEnsemble = getCurrentEnsemble();
	replaced = EnsembleUtils.diffEnsemble(origEnsemble, newEnsemble);
	LOG.info("New Ensemble: {} for ledger: {}", newEnsemble, ledgerId);

	changingEnsemble = false;
	}
	}
	if (newEnsemble != null) { // unsetSuccess outside of lock
	unsetSuccessAndSendWriteRequest(newEnsemble, replaced);
	}
	}
	}, clientCtx.getMainWorkerPool().chooseThread(ledgerId));

Fix NPE issue during recovery add #3620

Are you sure you want to change the base?

Fix NPE issue during recovery add #3620

Conversation

zhaohaidao commented Nov 7, 2022

Motivation

Changes

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

hangc0276 left a comment

Choose a reason for hiding this comment

zhaohaidao commented Nov 8, 2022 • edited Loading

zhaohaidao commented Nov 13, 2022

hangc0276 commented Jan 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhaohaidao commented Jan 20, 2023

horizonzy commented Jan 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhaohaidao commented Jan 20, 2023

zhaohaidao commented Jan 20, 2023

zhaohaidao commented Jan 20, 2023

zhaohaidao commented Jan 28, 2023

dlg99 left a comment

Choose a reason for hiding this comment

zhaohaidao commented Sep 16, 2023

zhaohaidao commented Sep 16, 2023

zhaohaidao commented Oct 9, 2023

zhaohaidao commented Oct 11, 2023

zhaohaidao commented Oct 11, 2023

zhaohaidao commented Oct 23, 2023

zhaohaidao commented Nov 8, 2022 •

edited

Loading