Remove a race condition that caused done to sometimes be executed twice …

[dreid]

…for the same set of input metrics.

Given two threads in a race and an initial count of 2.

• t0 - Thread1 calls shortLatch.countDown() (count = 1)
• t1 - Thread2 calls shortLatch.countDown() (count = 0)
• t2 - Thread1 calls done()
• t3 - Thread2 calls done()
• t4 - Thread1 calls shortLatch.getCount() (count = 0)
• t5 - Thread1 calls shortLatch.getCount() (count = 0)

As you can see in this scenario both threads will execute the body of the if statement in the done().

The below pull request address this issue by looking at what the if statement is actually doing (and what the latch is actually guarding against.)

It appears that aside from stopping actualWriteCtx which should only be done once, the other conditions apply to individual batches and the latch was used just to prevent logging more than one similar event.

I don't think there is very much value in that, so I've moved them inline to the work (thus also removing AtomicBoolean) and moved the stopping of actualWriteCtx to a listener on the aggregate Future.

So it'll only be stopped once when all futures of all batches have a result.

[coveralls]

Coverage decreased (-0.02%) to 53.08% when pulling e50249b on dreid:batchwriter-latch-race into 742cd95 on rackerlabs:master.

[dreid]

The coverage changes coveralls is complaining about are in files I didn't touch.

[GeorgeJahad]

sameThreadExecutor() has been deprecated in favor of directExecutor(): http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/MoreExecutors.html#sameThreadExecutor()

Otherwise this looks good.

[dreid]

It looks like directExecutor was added in guava 18.0 and blueflood-core is using guava 15.0.

[GeorgeJahad]

I'm confused about the coveralls issue. Why is it complaining about AstyanaxReader and DataType when you didn't change them?

[dreid]

@GeorgeJahad I'm also confused by it. I think coveralls might be comparing my branch's coverage against the wrong baseline.

[GeorgeJahad]

+1