asafm commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r945631200
##########
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##########
@@ -112,10 +117,31 @@
/** The main purpose of state maintenance is to prevent written after
close. **/
private volatile State state;
- private static final AtomicReferenceFieldUpdater<TxnLogBufferedWriter,
TxnLogBufferedWriter.State> STATE_UPDATER =
- AtomicReferenceFieldUpdater
- .newUpdater(TxnLogBufferedWriter.class,
TxnLogBufferedWriter.State.class, "state");
+ private final BookKeeperBatchedWriteCallback
bookKeeperBatchedWriteCallback = new BookKeeperBatchedWriteCallback();
+
+ private final TxnLogBufferedWriterMetricsStats metrics;
+
+ /**
+ * In the {@link #asyncAddData}, exceptions may occur. To avoid losing the
callback, use a variable to mark whether
+ * a callback needs to be compensated.
+ */
+ private boolean shouldCallBackWhenWriteFail;
Review Comment:
That's a very big no-no unless I missed something.
1. That's like the anti-pattern of defining a global variable.
2. Look at the distances - you're declaring the variable here, updating it
in the public add data method, and also updating it inside the internal add
data method. That's too messy and hard to keep up.
3. Your red flag should be raised pretty high when you need 2 full sentences
which I honestly don't understand as a comment for a single variable.
I suggest:
1. Delete variable from here
2. Use the option to return a value for the internal add async method to
return that value
--
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]
