Add user-defined properties to cursor position#744
Conversation
merlimat
added
the
type/feature
merlimat
force-pushed
the
cursor-properties
branch
from
aab6754 to
7029fc5
Compare
|
(Replying here since I don't know why this doesn't show up on github)
The lastMarkDeleteEntry is important to guarantee that we are storing the
same couple of (mark-delete-pos, properties) together, even when we are
rolling over the ledger.
In many cases we would be picking up the last mark-delete op, though now we
need to ensure they're together.
On Wed, Sep 6, 2017 at 5:59 PM Rajan Dhabalia ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
<#744 (comment)>
:
> @@ -87,9 +90,10 @@
protected volatile PositionImpl markDeletePosition;
protected volatile PositionImpl readPosition;
+ private volatile PendingMarkDeleteEntry lastMarkDeleteEntry;
is it necessary to introduce lastMarkDeleteEntry field in this PR?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#744 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAD0JG7wyCAgrpVxVJr2iMToIqT1-2qgks5sf0AAgaJpZM4POqwn>
.
--
Matteo Merli
<mmerli@apache.org>
|
|
|
||
| protected volatile PositionImpl markDeletePosition; | ||
| protected volatile PositionImpl readPosition; | ||
| private volatile PendingMarkDeleteEntry lastMarkDeleteEntry; |
There was a problem hiding this comment.
is it necessary to introduce lastMarkDeleteEntry field in this PR?
The lastMarkDeleteEntry is important to guarantee that we are storing the
same couple of (mark-delete-pos, properties) together, even when we are
rolling over the ledger.
Yes, I removed comment to understand more. But using class PendingMarkDeleteEntry for markDeleteEntry is little confusing and hard to set the context. Should we create dedicated sub-class to store this tuple?
There was a problem hiding this comment.
The PendingMarkDeleteEntry is only a container for position plus other fields. I guess we could rename it into MarkDeleteEntry or similar.
There was a problem hiding this comment.
I guess we could rename it into MarkDeleteEntry or similar.
Yes, renaming would be also fine.
|
|
||
| try { | ||
| internalAsyncMarkDelete(newMarkDeletePosition, new MarkDeleteCallback() { | ||
| internalAsyncMarkDelete(newMarkDeletePosition, Collections.emptyMap(), new MarkDeleteCallback() { |
There was a problem hiding this comment.
we are creating map even in case we don't need to store properties for cursor. Can we store it as null/Optional to avoid additional object creation (yes, it will add additional validation for null everywhere).?
There was a problem hiding this comment.
Collections.emptyMap() always return the same static instance of an empty map.
|
@rdhabalia Renamed |
2 participants
Motivation
Add user-defined properties that can be attached to a particular cursor position. The properties allows to attach store atomically some metadata along with a cursor position.
Wiki proposal with design doc:
https://github.com/apache/incubator-pulsar/wiki/PIP-6:-Guaranteed-Message-Deduplication
Modifications
ManagedCursorprotobuf definitionmarkDelete()operations now accepts aMap<String, Long>that will be stored with the positionManagedCursor.getProperties()to retrieve the properties after cursor recovery