You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by "Flavio Junqueira (JIRA)" <ji...@apache.org> on 2011/09/19 23:09:08 UTC
[jira] [Created] (BOOKKEEPER-68) Conditional setData
Conditional setData
-------------------
Key: BOOKKEEPER-68
URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
Project: Bookkeeper
Issue Type: Bug
Reporter: Flavio Junqueira
Assignee: Flavio Junqueira
The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (BOOKKEEPER-68) Conditional setData
Posted by "Flavio Junqueira (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Junqueira updated BOOKKEEPER-68:
---------------------------------------
Attachment: BOOKKEEPER-68.patch
After making the modifications that Ivan suggested, I noticed while running the tests that BookieRecoveryTest was failing intermittently, so I investigated it a bit and found the following.
Currently when we recover a ledger, we acknowledge the open before waiting for the ledger to close. Not waiting was causing a race while recovering bookies. I have fixed the problem in the patch I'm attaching.
There is one tiny thing that is bugging me in this patch and I need to fix. I need to access the znodeVersion variable from BookKeeperTools, and to do it I made the variable public just to test. I need to find a better way of accessing the znode version variable in LedgerMetadata, though.
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Fix For: 3.4.0
>
> Attachments: BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (BOOKKEEPER-68) Conditional setData
Posted by "Flavio Junqueira (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Junqueira updated BOOKKEEPER-68:
---------------------------------------
Attachment: BOOKKEEPER-68.patch
Posted the diff on the review board.
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Fix For: 3.4.0
>
> Attachments: BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (BOOKKEEPER-68) Conditional setData
Posted by "Flavio Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Junqueira updated BOOKKEEPER-68:
---------------------------------------
Attachment: BOOKKEEPER-68.patch
Preliminary patch (no new test, but current tests pass).
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Attachments: BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-68) Conditional setData
Posted by "Ivan Kelly (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108746#comment-13108746 ]
Ivan Kelly commented on BOOKKEEPER-68:
--------------------------------------
Im not sure. I think that situations like this should have an external mechanism. I've actually implemented a bit of the handling for BadVersion. It'll need a lot of testing though.
https://github.com/ivankelly/bookkeeper/commit/7b2d2f9163afb735e73e3e285f2d358bb4b71fda
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Attachments: BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (BOOKKEEPER-68) Conditional setData
Posted by "Flavio Junqueira (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Junqueira updated BOOKKEEPER-68:
---------------------------------------
Attachment: BOOKKEEPER-68.patch
Patch with test. In the end, the test was way easier to write than I expected. I simply create a reader before the writer closes the ledger, and I check that the writer call to close the ledger fails.
To be able to verify that the call to close has failed, I needed to change the signature of the close call, which was returning void. Hope no one finds it to be a big deal.
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Fix For: 3.4.0
>
> Attachments: BOOKKEEPER-68.patch, BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (BOOKKEEPER-68) Conditional setData
Posted by "Flavio Junqueira (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Junqueira updated BOOKKEEPER-68:
---------------------------------------
Attachment: BOOKKEEPER-68.patch
Fixed the last point on the return code that Ivan raised on the review board. -F
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Fix For: 3.4.0
>
> Attachments: BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-68) Conditional setData
Posted by "Ivan Kelly (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129596#comment-13129596 ]
Ivan Kelly commented on BOOKKEEPER-68:
--------------------------------------
Committed r1185532, thanks Flavio.
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Fix For: 3.4.0
>
> Attachments: BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch, BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-68) Conditional setData
Posted by "Ivan Kelly (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13112411#comment-13112411 ]
Ivan Kelly commented on BOOKKEEPER-68:
--------------------------------------
The approach I had is at:
https://github.com/ivankelly/bookkeeper/tree/Fencing
Basically, if you fail to update, you jump back to the very start of recovery. The recovery operation is idempotent in this case, so it will succeed in opening for reading. For NN, there would be another mechanism to ensure that only one ledger is open for writing at a time, as really that's where we want collisions to happen if they do happen.
Flavio made a good point yesterday that multiple concurrent recoveries will steal bandwidth from each other though, which I think is the best argument for erroring one of them out.
Actually, the BKJournal for NN is available at:
https://github.com/ivankelly/hadoop-common/tree/BKJournal
if you want to take a look.
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Attachments: BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-68) Conditional setData
Posted by "Flavio Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13109832#comment-13109832 ]
Flavio Junqueira commented on BOOKKEEPER-68:
--------------------------------------------
I was also thinking that leaving to the application gives the opportunity to the client to give up. One case I can think of in which not retrying might be better is this. We have two cold backup servers, and both try to become the primary. They concurrently start to read from bookkeeper and, in the process, they try to recover the ledger. If one fails to recover because of the concurrent write of the ledger metadata, then it is better to have one replica giving up, since it will leave more read bandwidth free to the other backup server.
I need to point out that I think both approaches are correct, and the argument I'm making is based on performance. My conclusion is that I don't feel strongly about having one way or the other, but I'm slightly inclined to leaving it to the application.
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Attachments: BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-68) Conditional setData
Posted by "Flavio Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108627#comment-13108627 ]
Flavio Junqueira commented on BOOKKEEPER-68:
--------------------------------------------
I'd rather leave to the application to decide because of the following argument. Suppose that two clients try to recover the ledger because are on the way of taking over the role of primary. The fact that one attempt to open the ledger and recover fails indicates that there has a been a concurrent attempt, and in this case I would say that the client that gets a failed request should actually back off.
Does it make sense to you, Ivan?
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Attachments: BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-68) Conditional setData
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13112216#comment-13112216 ]
dhruba borthakur commented on BOOKKEEPER-68:
--------------------------------------------
I am more inclined to say that for the use case of NN using BK, it makes sense to make one of the standby namenodes to fail the recover-ledger api call (rather than retrying inside the BK client). The standby namenode will go back to ZK to see if a new namenode master has been selected, if not then the standby namenode can retry.
Ivan: is there some other approach that you have in mind?
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Attachments: BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-68) Conditional setData
Posted by "Ivan Kelly (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108620#comment-13108620 ]
Ivan Kelly commented on BOOKKEEPER-68:
--------------------------------------
This patch seems to just cause the open ledger to fail. Shouldn't the failing one just retry to read the metadata?
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Attachments: BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-68) Conditional setData
Posted by "Flavio Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108761#comment-13108761 ]
Flavio Junqueira commented on BOOKKEEPER-68:
--------------------------------------------
I think you're right in that we would need such an external mechanism anyway because the absence of a failure does not imply that it is ok to take over in my example. Consequently, retrying does not make it more difficult for the application.
I still feel that the code is simpler without retrying, though, and it doesn't sound like a big deal for the application to retry if it needs to. At the same time, I'm possibly not the best person to ask about APIs, so I'm willing to change my mind here if there is a good reason for doing it.
> Conditional setData
> -------------------
>
> Key: BOOKKEEPER-68
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-68
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Flavio Junqueira
> Assignee: Flavio Junqueira
> Attachments: BOOKKEEPER-68.patch
>
>
> The write to ZooKeeper to store ledger metadata when closing a ledger must be conditional, otherwise concurrent clients might end up writing in a way that the update of a client overwrites the update of the other.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira