eolivelli commented on a change in pull request #10790:
URL: https://github.com/apache/pulsar/pull/10790#discussion_r648138748
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
##########
@@ -285,6 +285,37 @@ public MarkDeleteEntry(PositionImpl newPosition,
Map<String, Long> properties,
return lastMarkDeleteEntry != null ? lastMarkDeleteEntry.properties :
Collections.emptyMap();
}
+ @Override
+ public boolean putProperty(String key, Long value) {
+ if (lastMarkDeleteEntry != null) {
+ MarkDeleteEntry currentLastMarkDeleteEntry = lastMarkDeleteEntry;
+ Map<String, Long> properties =
currentLastMarkDeleteEntry.properties;
+ if (properties == null || properties.isEmpty()) {
Review comment:
do we have concurrency issues here ?
there is no lock or other synchronisation primitive that prevents two calls
to access this variable, the same happens for `lastMarkDeleteEntry`
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
##########
@@ -285,6 +285,37 @@ public MarkDeleteEntry(PositionImpl newPosition,
Map<String, Long> properties,
return lastMarkDeleteEntry != null ? lastMarkDeleteEntry.properties :
Collections.emptyMap();
}
+ @Override
+ public boolean putProperty(String key, Long value) {
+ if (lastMarkDeleteEntry != null) {
+ MarkDeleteEntry currentLastMarkDeleteEntry = lastMarkDeleteEntry;
+ Map<String, Long> properties =
currentLastMarkDeleteEntry.properties;
+ if (properties == null || properties.isEmpty()) {
+ Map<String, Long> newProperties = Maps.newHashMap();
+ newProperties.put(key, value);
+ lastMarkDeleteEntry = new
MarkDeleteEntry(currentLastMarkDeleteEntry.newPosition, newProperties,
+ currentLastMarkDeleteEntry.callback,
currentLastMarkDeleteEntry.ctx);
+ lastMarkDeleteEntry.callbackGroup =
currentLastMarkDeleteEntry.callbackGroup;
+ } else {
+ properties.put(key, value);
+ }
+ return true;
+ }
+ return false;
Review comment:
I do not understand this part of the story.
Probably I miss some context.
if `lastMarkDeleteEntry` is null, then the command in finally ignored, we
are not storing anyway the information requested by the user.
if this is correct, then I believe that we must fail the API call, because
otherwise the user will think that the change is applied but this is not true.
Can you please clarify ?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
