Flavio Paiva Junqueira commented on ZOOKEEPER-373:

> the one trick piece is the refCounting. specifically decrementing 
> the counter. in close when you decrement the counter and then 
> do the remove you aren't in a synchronized method, so you have 
> a race condition.

That was a great catch. I have initially thought that making the counter 
volatile was sufficient given the code paths that we have, but I realized after 
your comment that this is not true, so I have re-structured a bit so that we 
don't have a race condition. 

I haven't made BookieHandle::incRefCount and BookieHandle::halt synchronized 
because they are currently only called from BookKeeper::getBookieHandle and 
BookKeeper::haltBookieHandles, respectively, and these two last methods are 
synchronized. To make it safe for future versions of BookKeeper, we could make 
the two BookieHandle methods synchronized, but if I'm not mistaken, I don't 
think they have to be in the way I have implemented. It would be great if you 
could double-check. 

> one other place that could be problematic is the mac object. 
> isn't it possible that two different threads going to two different
> ledgers use the same mac?

This one doesn't seem to be a problem because each mac object is a variable of 
either BookieHandle (for writes) or BookieClient (for reads), and each of these 
classes extends Thread. Finally, the methods that use the mac objects are 
called from the Threads that own them. 

> One thread per bookie
> ---------------------
>                 Key: ZOOKEEPER-373
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-373
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: contrib-bookkeeper
>    Affects Versions: 3.1.1
>            Reporter: Flavio Paiva Junqueira
>            Assignee: Flavio Paiva Junqueira
>             Fix For: 3.2.0
>         Attachments: ZOOKEEPER-373.patch, ZOOKEEPER-373.patch, 
> ZOOKEEPER-373.patch
> Currently, if a client is writing to multiple ledgers and these ledgers 
> overlap on some bookie, there will be as many threads for such a bookie as 
> the number of ledgers writing to it. Consequently, if a client writes to many 
> ledgers simultaneously, it may end up with an undesirably large number of 
> threads. I don't have a concrete proposal yet, but I suspect it is as simple 
> as keeping an array of BookieHandle objects, one per bookie, and having each 
> BookieHandle object shared by all ledgers.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to