You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by sijie <gi...@git.apache.org> on 2017/06/21 22:23:54 UTC

[GitHub] bookkeeper pull request #200: BookKeeper Client: Speculative Read/Read LAC p...

GitHub user sijie opened a pull request:

    https://github.com/apache/bookkeeper/pull/200

    BookKeeper Client: Speculative Read/Read LAC policy

    Descriptions of the changes in this PR:
    
        - Separate out the logic that determines when speculative reads are scheduled from the individual requests into a separate SpeculativeRequestExecutionPolicy
        - Implement the default speculative read execution policy based on the current mechanism of using first and max timeout
        - Initialize the policy once in the BookKeeper object and use it for individual requests by keeping information about the requests in the call chain.
    
    ---
    Be sure to do all of the following to help us incorporate your contribution
    quickly and easily:
    
    - [x] Make sure the PR title is formatted like:
        `<Issue #>: Description of pull request`
        `e.g. Issue 123: Description ...`
    - [x] Make sure tests pass via `mvn clean apache-rat:check install findbugs:check`.
    - [x] Replace `<Issue #>` in the title with the actual Issue number, if there is one.
    
    ---


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sijie/bookkeeper client_changes/speculative_policy

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/bookkeeper/pull/200.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #200
    
----
commit 4b192f406aef57fbccc198f56820e30550368298
Author: Sijie Guo <si...@twitter.com>
Date:   2014-09-09T20:24:28Z

    Handle duplicated bookies on read entry or read lac with entry
    
    * change the HashSet to BitSet to use bookieIndex rather than InetSocketAddress to track the heardfrom bookies set.
    
    RB_ID=449651

commit e79d62032af83bbd70dae1067768e81bb106677e
Author: Robin Dhamankar <rd...@twitter.com>
Date:   2014-11-04T01:22:35Z

    BookKeeper Client: Speculative Read/Read LAC policy
    
    - Separate out the logic that determines when speculative reads are scheduled from the individual requests into a separate SpeculativeRequestExecutionPolicy
    - Implement the default speculative read execution policy based on the current mechanism of using first and max timeout
    - Initialize the policy once in the BookKeeper object and use it for individual requests by keeping information about the requests in the call chain.
    
    RB_ID=501463

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bookkeeper issue #200: Issue 209: Introduce Speculative Read/Read LAC policy

Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:

    https://github.com/apache/bookkeeper/pull/200
  
    - in the upcoming changes of long poll requests, we need same speculative policy for long poll requests. abstract the logic as a policy will make things easier and we don't have to duplicate the code for both reads and long poll reads
    - abstracting the logic as a policy will help us improving the policy without changing the read code.
    - we also extend the default policy to have exponential backup in place.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bookkeeper issue #200: BookKeeper Client: Speculative Read/Read LAC policy

Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:

    https://github.com/apache/bookkeeper/pull/200
  
    This pull request is based on #199 ( e79d620 is the change for this pull request ).
    
    The change is contributed by Robin Dhamankar


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bookkeeper issue #200: BookKeeper Client: Speculative Read/Read LAC policy

Posted by jiazhai <gi...@git.apache.org>.
Github user jiazhai commented on the issue:

    https://github.com/apache/bookkeeper/pull/200
  
    Would you please open an Issue for it, and change the name of this PR as "Issue XXX: Description ...." ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bookkeeper issue #200: BookKeeper Client: Speculative Read/Read LAC policy

Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:

    https://github.com/apache/bookkeeper/pull/200
  
    @sijie in https://cwiki.apache.org/confluence/display/BOOKKEEPER/BP-9+-+Github+issues+for+Issue+Tracking
    @jiazhai  wrote "Always create an issue for a pull request" as @fpj suggested.
    
    I think we all agreed to what was written in the wiki
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bookkeeper issue #200: BookKeeper Client: Speculative Read/Read LAC policy

Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:

    https://github.com/apache/bookkeeper/pull/200
  
    @jiazhai I think we agreed on the workflow - if there is already a change, people can submit a pull request directly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bookkeeper pull request #200: Issue 209: Introduce Speculative Read/Read LAC...

Posted by sijie <gi...@git.apache.org>.
Github user sijie closed the pull request at:

    https://github.com/apache/bookkeeper/pull/200


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bookkeeper issue #200: BookKeeper Client: Speculative Read/Read LAC policy

Posted by jiazhai <gi...@git.apache.org>.
Github user jiazhai commented on the issue:

    https://github.com/apache/bookkeeper/pull/200
  
    @eolivelli, thanks for the info.  @sijie, excuse me, there maybe some mis-sync regarding this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bookkeeper issue #200: Issue 209: Introduce Speculative Read/Read LAC policy

Posted by jvrao <gi...@git.apache.org>.
Github user jvrao commented on the issue:

    https://github.com/apache/bookkeeper/pull/200
  
    @sijie what is the major drive of this change? Current method of trying speculative reads has any major flaw? I understand this is better organized as speculative reads has its own policy, but trying to understand what use-case drove you to this change at Twitter?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---