dlg99 commented on code in PR #15757:
URL: https://github.com/apache/pulsar/pull/15757#discussion_r880803341
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -845,11 +845,12 @@ public ManagedCursor openCursor(String cursorName) throws
InterruptedException,
@Override
public ManagedCursor openCursor(String cursorName, InitialPosition
initialPosition)
throws InterruptedException, ManagedLedgerException {
- return openCursor(cursorName, initialPosition, Collections.emptyMap());
+ return openCursor(cursorName, initialPosition, Collections.emptyMap(),
Collections.emptyMap());
Review Comment:
we may be better of with something like `final static Map<blah, blah> =
Maps.ImmutableMap();` as a field to avoid creating empty maps adding to
short-lived memory garbage.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -361,6 +369,18 @@ public void operationComplete(ManagedCursorInfo info, Stat
stat) {
cursorLedgerStat = stat;
lastActive = info.getLastActive() != 0 ? info.getLastActive()
: lastActive;
+
+ Map<String, String> recoveredCursorProperties =
Collections.emptyMap();
+ if (info.getCursorPropertiesCount() > 0) {
+ // Recover properties map
+ recoveredCursorProperties = Maps.newHashMap();
Review Comment:
nit: in 4 lines here (and in some other places in the change) we have two
ways of creating new map (`Collections.emptyMap()` and `Maps.newHashMap()`).
While nothing is wrong with either one having both requires reviewer to
think about motivation for having two, differences, etc. Do we really need both?
--
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]
