BOOKKEEPER-965: Long Polling Part I: Changes in the write path #73
Conversation
|
LGTM +1 @jvrao this is our first change about the long poll read request. I know you guys are also working on the lac related change. Can you also take a look this change to make sure it will be compatible with your change. |
|
This directly corresponds to what we have been using and have local changes. You are aware of our change. Is this something you worked on recently? if so, I am wondering why to reinvent the wheel instead of using our patch. |
|
@jvrao no, this is the old change at Twitter's branch. we are merging these changes back. |
| @@ -80,6 +80,9 @@ | |||
| private int stateBits; | |||
| private boolean needFlushHeader = false; | |||
|
|
|||
| // lac | |||
| private Long lac = null; | |||
There was a problem hiding this comment.
Is this used as an optimization to quickly find the highest LAC sent by the writer?
There was a problem hiding this comment.
yes. it will also be used for the subsequent long poll change. so we can detect the lac changes and satisfy any long poll wait requests.
|
@jvrao if there is no objection from you, I'd like to merge this and move on sending the subsequent requests. |
|
@sijie Sorry responding after it is pulled in. Do you know how-much savings you are getting with this? I have changes in similar lines but to keep explicitLac in FileInfo. Read path may be little more complicated. I will watch out for it. In the mean time I will push my changes too. |
|
@jvrao what do you mean saving? you mean the long poll change? |
|
@sijie Saving = perf benefit. |
|
@jvrao gotcha. we didn't measure that in particularly. the most benefit we have with this is we can have long poll requests satisfied when lac is advanced. FileInfo is the best place to us to implement this feature. |
This is the first in the series of changes for enabling long polling between bookkeeper client and the bookkeeper server. The changes were originally implemented in the Twitter fork and these pull request combine multiple commits from Twitter's bookkeeper fork as they include not only the changes made initially but also bug fixes added since. The first change captures the changes on the write path (AddEntry). We track the last add confirmed in the FileInfo so that we can trigger actions when the value is updated Author: Robin Dhamankar <robindh@Robins-MacBook-Air.local> Reviewers: sijie@apache.org <sijie@apache.org> Closes apache#73 from robindh/LongPollWritePath
Author: xieliang <xieliang007@gmail.com> Reviewers: Sijie Guo <sijie@apache.org>, Leigh Stewart <lstewart@apache.org> Closes apache#73 from xieliang/DL-28
This is the first in the series of changes for enabling long polling between bookkeeper client and the bookkeeper server. The changes were originally implemented in the Twitter fork and these pull request combine multiple commits from Twitter's bookkeeper fork as they include not only the changes made initially but also bug fixes added since.
The first change captures the changes on the write path (AddEntry). We track the last add confirmed in the FileInfo so that we can trigger actions when the value is updated