michaeljmarshall commented on code in PR #15067:
URL: https://github.com/apache/pulsar/pull/15067#discussion_r848638415
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -1793,6 +1830,34 @@ protected void internalAsyncMarkDelete(final
PositionImpl newPosition, Map<Strin
}
void internalMarkDelete(final MarkDeleteEntry mdEntry) {
+ if (persistentMarkDeletePosition != null
+ && mdEntry.newPosition.compareTo(persistentMarkDeletePosition)
< 0) {
+ if (log.isInfoEnabled()) {
+ log.info("Skipping updating mark delete position to {}. The
persisted mark delete position {} "
+ + "is latest.", mdEntry.newPosition,
persistentMarkDeletePosition);
+ }
+ mdEntry.triggerComplete();
+ return;
+ }
+
+ PositionImpl inProgressLatest =
INPROGRESS_MARKDELETE_PERSIST_POSITION_UPDATER.updateAndGet(this, current -> {
+ if (current != null && current.compareTo(mdEntry.newPosition) > 0)
{
+ return current;
+ } else {
+ return mdEntry.newPosition;
+ }
+ });
+
+ // if there's a newer or equal mark delete update in progress, skip it.
+ if (inProgressLatest != mdEntry.newPosition) {
+ if (log.isInfoEnabled()) {
+ log.info("Skipping updating mark delete position to {}. The
mark delete position update "
+ + "in progress {} is latest.", mdEntry.newPosition,
inProgressLatest);
Review Comment:
Nit: I think we might want to replace `latest` with `later` because `latest`
already has a meaning for a topic.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -1793,6 +1830,34 @@ protected void internalAsyncMarkDelete(final
PositionImpl newPosition, Map<Strin
}
void internalMarkDelete(final MarkDeleteEntry mdEntry) {
+ if (persistentMarkDeletePosition != null
+ && mdEntry.newPosition.compareTo(persistentMarkDeletePosition)
< 0) {
+ if (log.isInfoEnabled()) {
+ log.info("Skipping updating mark delete position to {}. The
persisted mark delete position {} "
+ + "is latest.", mdEntry.newPosition,
persistentMarkDeletePosition);
Review Comment:
Nit: same comment about replacing `latest` with `later`.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -1793,6 +1830,34 @@ protected void internalAsyncMarkDelete(final
PositionImpl newPosition, Map<Strin
}
void internalMarkDelete(final MarkDeleteEntry mdEntry) {
+ if (persistentMarkDeletePosition != null
+ && mdEntry.newPosition.compareTo(persistentMarkDeletePosition)
< 0) {
+ if (log.isInfoEnabled()) {
+ log.info("Skipping updating mark delete position to {}. The
persisted mark delete position {} "
+ + "is latest.", mdEntry.newPosition,
persistentMarkDeletePosition);
+ }
+ mdEntry.triggerComplete();
+ return;
+ }
+
+ PositionImpl inProgressLatest =
INPROGRESS_MARKDELETE_PERSIST_POSITION_UPDATER.updateAndGet(this, current -> {
+ if (current != null && current.compareTo(mdEntry.newPosition) > 0)
{
+ return current;
+ } else {
+ return mdEntry.newPosition;
+ }
+ });
+
+ // if there's a newer or equal mark delete update in progress, skip it.
+ if (inProgressLatest != mdEntry.newPosition) {
+ if (log.isInfoEnabled()) {
+ log.info("Skipping updating mark delete position to {}. The
mark delete position update "
+ + "in progress {} is latest.", mdEntry.newPosition,
inProgressLatest);
+ }
+ mdEntry.triggerComplete();
Review Comment:
Is it correct to complete this `MarkDeleteEntry`? It seems to me that it's
ambiguous at this point in the code because the current `inProgressLatest`
might be in the process of persisting that position as mark deleted. I'd think
we should comply complete that `MarkDeleteEntry` once the in progress mark
delete is completed.
--
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]
