You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Ivan Kelly <iv...@apache.org> on 2012/10/26 19:54:24 UTC
Review Request: BOOKKEEPER-336 bookie readEntries is taking more time if the
ensemble has failed bookie(s)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7749/
-----------------------------------------------------------
Review request for bookkeeper.
Description
-------
Scenario:
1) Start three bookies. Create ledger with ensemblesize=3, quorumsize=2
2) Add 100 entries to this ledger
3) Make first bookie down and read the entries from 0-99
Output: Each entry is going to fetch from the failed bookie and is waiting for the bookie connection timeout, only after failure going to next bookie.
This is affecting the read entry performance.
Impact: Namenode switching time will be affected by adding this failed bookie readTimeOut also.
This addresses bug BOOKKEEPER-336.
https://issues.apache.org/jira/browse/BOOKKEEPER-336
Diffs
-----
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java aeb6886
Diff: https://reviews.apache.org/r/7749/diff/
Testing
-------
Thanks,
Ivan Kelly
Re: Review Request: BOOKKEEPER-336 bookie readEntries is taking more time if
the ensemble has failed bookie(s)
Posted by Ivan Kelly <iv...@apache.org>.
> On Oct. 28, 2012, 7:42 p.m., Rakesh R wrote:
> > Thanks Ivan for the patch. There is few compilation errors 'lh.getLedgerMetadata().getQuorumSize()'. could you please rebase it in latest trunk.
> >
> > Just few thoughts:
> >
> > comment#1:
> > getSentHosts() method not required to be synchronized, its already being called from a sync method maybeSendSpeculativeRead()
> >
> > comment#2:
> > }, 2000, 2000); Can we make this timer interval configurable and defaulting to 2000.
> >
> > comment#3:
> > How about scheduling the speculativeReadTimer after sending all the request, so will be able to avoid boolean flag 'requestsSent' usage?
> >
> > comment#4:
> > Please make the ReadContext class to 'private static class ReadContext'
> >
> > comment#5:
> > + BitSet b = new BitSet(ensemble.size());
> > +
> > + for (int i = 0; i < b.length(); i++) {
> > + if (b.get(i)) {
> > + b.set(lh.distributionSchedule.getBookieIndex(entryId, i));
> > + }
> > + }
> > Here by default all the bitset values will be init to false, so here b.set(lh.distributionSchedule.getBookieIndex(entryId, i)); will be unreachable. isn't it?
> >
> > comment#6
> > Could you please add the "heardFromHosts.add(rctx.to);" after checking 'entry.complete(rctx.to, buffer)'.
> > Consider the case where read entry throws BKDigestMatchException and the entry still needs retryals. But the timer sees a heardFromHosts value and it will stop sending next read.
I uploaded the wrong patch on friday. I had already addressed 2, 3 & 5.
I've fixed 1 & 3.
For 6, I think it's fine as it is. heardFromHosts is only there for speculative reads, and the fact that we received an entry, even if its corrupt signifies that the host is up. If the response is corrupt, logErrorAndReattemptRead will send a new read request anyhow.
- Ivan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7749/#review12866
-----------------------------------------------------------
On Oct. 26, 2012, 5:54 p.m., Ivan Kelly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7749/
> -----------------------------------------------------------
>
> (Updated Oct. 26, 2012, 5:54 p.m.)
>
>
> Review request for bookkeeper.
>
>
> Description
> -------
>
> Scenario:
>
> 1) Start three bookies. Create ledger with ensemblesize=3, quorumsize=2
> 2) Add 100 entries to this ledger
> 3) Make first bookie down and read the entries from 0-99
>
> Output: Each entry is going to fetch from the failed bookie and is waiting for the bookie connection timeout, only after failure going to next bookie.
> This is affecting the read entry performance.
>
> Impact: Namenode switching time will be affected by adding this failed bookie readTimeOut also.
>
>
> This addresses bug BOOKKEEPER-336.
> https://issues.apache.org/jira/browse/BOOKKEEPER-336
>
>
> Diffs
> -----
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java aeb6886
>
> Diff: https://reviews.apache.org/r/7749/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ivan Kelly
>
>
Re: Review Request: BOOKKEEPER-336 bookie readEntries is taking more time if
the ensemble has failed bookie(s)
Posted by Rakesh R <ra...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7749/#review12866
-----------------------------------------------------------
Thanks Ivan for the patch. There is few compilation errors 'lh.getLedgerMetadata().getQuorumSize()'. could you please rebase it in latest trunk.
Just few thoughts:
comment#1:
getSentHosts() method not required to be synchronized, its already being called from a sync method maybeSendSpeculativeRead()
comment#2:
}, 2000, 2000); Can we make this timer interval configurable and defaulting to 2000.
comment#3:
How about scheduling the speculativeReadTimer after sending all the request, so will be able to avoid boolean flag 'requestsSent' usage?
comment#4:
Please make the ReadContext class to 'private static class ReadContext'
comment#5:
+ BitSet b = new BitSet(ensemble.size());
+
+ for (int i = 0; i < b.length(); i++) {
+ if (b.get(i)) {
+ b.set(lh.distributionSchedule.getBookieIndex(entryId, i));
+ }
+ }
Here by default all the bitset values will be init to false, so here b.set(lh.distributionSchedule.getBookieIndex(entryId, i)); will be unreachable. isn't it?
comment#6
Could you please add the "heardFromHosts.add(rctx.to);" after checking 'entry.complete(rctx.to, buffer)'.
Consider the case where read entry throws BKDigestMatchException and the entry still needs retryals. But the timer sees a heardFromHosts value and it will stop sending next read.
- Rakesh R
On Oct. 26, 2012, 5:54 p.m., Ivan Kelly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7749/
> -----------------------------------------------------------
>
> (Updated Oct. 26, 2012, 5:54 p.m.)
>
>
> Review request for bookkeeper.
>
>
> Description
> -------
>
> Scenario:
>
> 1) Start three bookies. Create ledger with ensemblesize=3, quorumsize=2
> 2) Add 100 entries to this ledger
> 3) Make first bookie down and read the entries from 0-99
>
> Output: Each entry is going to fetch from the failed bookie and is waiting for the bookie connection timeout, only after failure going to next bookie.
> This is affecting the read entry performance.
>
> Impact: Namenode switching time will be affected by adding this failed bookie readTimeOut also.
>
>
> This addresses bug BOOKKEEPER-336.
> https://issues.apache.org/jira/browse/BOOKKEEPER-336
>
>
> Diffs
> -----
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java aeb6886
>
> Diff: https://reviews.apache.org/r/7749/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ivan Kelly
>
>
Re: Review Request: BOOKKEEPER-336 bookie readEntries is taking more time if
the ensemble has failed bookie(s)
Posted by Rakesh R <ra...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7749/#review13337
-----------------------------------------------------------
Ship it!
Ship It!
- Rakesh R
On Oct. 31, 2012, 5:57 p.m., Ivan Kelly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7749/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2012, 5:57 p.m.)
>
>
> Review request for bookkeeper.
>
>
> Description
> -------
>
> Scenario:
>
> 1) Start three bookies. Create ledger with ensemblesize=3, quorumsize=2
> 2) Add 100 entries to this ledger
> 3) Make first bookie down and read the entries from 0-99
>
> Output: Each entry is going to fetch from the failed bookie and is waiting for the bookie connection timeout, only after failure going to next bookie.
> This is affecting the read entry performance.
>
> Impact: Namenode switching time will be affected by adding this failed bookie readTimeOut also.
>
>
> This addresses bug BOOKKEEPER-336.
> https://issues.apache.org/jira/browse/BOOKKEEPER-336
>
>
> Diffs
> -----
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java c8e814c
> bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java 8443c27
> bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestSpeculativeRead.java PRE-CREATION
> bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java 2f39536
>
> Diff: https://reviews.apache.org/r/7749/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ivan Kelly
>
>
Re: Review Request: BOOKKEEPER-336 bookie readEntries is taking more time if
the ensemble has failed bookie(s)
Posted by Rakesh R <ra...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7749/#review13336
-----------------------------------------------------------
Great work Ivan. New patch looks pretty well and I've just one thought.
I've read the comment '// only send another read, if we have had no response at all (even for other entries)' just got following doubt.
Consider a scenario where reading n entries, after reading x entries one of the bookie present in 'heardFromHosts' is down. Does it sound to allow the next (n-x) entries go with timertask's speculative read rather than skipping. I'm seeing the condition if (sentTo.cardinality() == 0) prevents speculative reads. Am I missing anything?
- Rakesh R
On Oct. 31, 2012, 5:57 p.m., Ivan Kelly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7749/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2012, 5:57 p.m.)
>
>
> Review request for bookkeeper.
>
>
> Description
> -------
>
> Scenario:
>
> 1) Start three bookies. Create ledger with ensemblesize=3, quorumsize=2
> 2) Add 100 entries to this ledger
> 3) Make first bookie down and read the entries from 0-99
>
> Output: Each entry is going to fetch from the failed bookie and is waiting for the bookie connection timeout, only after failure going to next bookie.
> This is affecting the read entry performance.
>
> Impact: Namenode switching time will be affected by adding this failed bookie readTimeOut also.
>
>
> This addresses bug BOOKKEEPER-336.
> https://issues.apache.org/jira/browse/BOOKKEEPER-336
>
>
> Diffs
> -----
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java c8e814c
> bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java 8443c27
> bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestSpeculativeRead.java PRE-CREATION
> bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java 2f39536
>
> Diff: https://reviews.apache.org/r/7749/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ivan Kelly
>
>
Re: Review Request: BOOKKEEPER-336 bookie readEntries is taking more time if
the ensemble has failed bookie(s)
Posted by Ivan Kelly <iv...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7749/
-----------------------------------------------------------
(Updated Oct. 31, 2012, 5:57 p.m.)
Review request for bookkeeper.
Changes
-------
new patch switches back where the timer is started, but the requestsSent flag wasnt needed anyhow. Also, there was a possible ConcurrentModificationException. Fixed by switching to ArrayBlockingQueue
Description
-------
Scenario:
1) Start three bookies. Create ledger with ensemblesize=3, quorumsize=2
2) Add 100 entries to this ledger
3) Make first bookie down and read the entries from 0-99
Output: Each entry is going to fetch from the failed bookie and is waiting for the bookie connection timeout, only after failure going to next bookie.
This is affecting the read entry performance.
Impact: Namenode switching time will be affected by adding this failed bookie readTimeOut also.
This addresses bug BOOKKEEPER-336.
https://issues.apache.org/jira/browse/BOOKKEEPER-336
Diffs (updated)
-----
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java c8e814c
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java 8443c27
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestSpeculativeRead.java PRE-CREATION
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java 2f39536
Diff: https://reviews.apache.org/r/7749/diff/
Testing
-------
Thanks,
Ivan Kelly
Re: Review Request: BOOKKEEPER-336 bookie readEntries is taking more time if
the ensemble has failed bookie(s)
Posted by Ivan Kelly <iv...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7749/
-----------------------------------------------------------
(Updated Oct. 31, 2012, 3:59 p.m.)
Review request for bookkeeper.
Description
-------
Scenario:
1) Start three bookies. Create ledger with ensemblesize=3, quorumsize=2
2) Add 100 entries to this ledger
3) Make first bookie down and read the entries from 0-99
Output: Each entry is going to fetch from the failed bookie and is waiting for the bookie connection timeout, only after failure going to next bookie.
This is affecting the read entry performance.
Impact: Namenode switching time will be affected by adding this failed bookie readTimeOut also.
This addresses bug BOOKKEEPER-336.
https://issues.apache.org/jira/browse/BOOKKEEPER-336
Diffs (updated)
-----
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java c8e814c
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java 8443c27
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestSpeculativeRead.java PRE-CREATION
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java 2f39536
Diff: https://reviews.apache.org/r/7749/diff/
Testing
-------
Thanks,
Ivan Kelly
Re: Review Request: BOOKKEEPER-336 bookie readEntries is taking more time if
the ensemble has failed bookie(s)
Posted by Ivan Kelly <iv...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7749/
-----------------------------------------------------------
(Updated Oct. 26, 2012, 5:54 p.m.)
Review request for bookkeeper.
Description
-------
Scenario:
1) Start three bookies. Create ledger with ensemblesize=3, quorumsize=2
2) Add 100 entries to this ledger
3) Make first bookie down and read the entries from 0-99
Output: Each entry is going to fetch from the failed bookie and is waiting for the bookie connection timeout, only after failure going to next bookie.
This is affecting the read entry performance.
Impact: Namenode switching time will be affected by adding this failed bookie readTimeOut also.
This addresses bug BOOKKEEPER-336.
https://issues.apache.org/jira/browse/BOOKKEEPER-336
Diffs
-----
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java aeb6886
Diff: https://reviews.apache.org/r/7749/diff/
Testing
-------
Thanks,
Ivan Kelly