Ready: Better Search Testing + Bug Fix

[alexmoore]

Refactored search setup into common base class to reduce duplication & make it easier to test search going forward.

Also contains a fix for #547 (CLIENTS-599), and #539 (CLIENTS-557).

[hazen]

I take it that the builderwas not creating anything for zero rows?

[hazen]

I think I see now. You changed the value to be initialized to -1 so now you have to set it to zero explicitly.

[hazen]

Errors from the builder:

Tests run: 20, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 18.191 sec <<< FAILURE! - in com.basho.riak.client.core.operations.itest.ITestSecondaryIndexQueryOp
testBucketIndexHack(com.basho.riak.client.core.operations.itest.ITestSecondaryIndexQueryOp)  Time elapsed: 1.01 sec  <<< FAILURE!
junit.framework.AssertionFailedError: null
    at junit.framework.Assert.fail(Assert.java:48)
    at junit.framework.Assert.assertTrue(Assert.java:20)
    at junit.framework.Assert.assertTrue(Assert.java:27)
    at com.basho.riak.client.core.operations.itest.ITestSecondaryIndexQueryOp.testBucketIndexHack(ITestSecondaryIndexQueryOp.java:540)

Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 2.241 sec <<< FAILURE! - in com.basho.riak.client.api.commands.indexes.itest.ITestRawIndexQuery
testBucketIndexHack(com.basho.riak.client.api.commands.indexes.itest.ITestRawIndexQuery)  Time elapsed: 0.787 sec  <<< FAILURE!
java.lang.AssertionError: expected:<100> but was:<103>
    at org.junit.Assert.fail(Assert.java:93)
    at org.junit.Assert.failNotEquals(Assert.java:647)
    at org.junit.Assert.assertEquals(Assert.java:128)
    at org.junit.Assert.assertEquals(Assert.java:472)
    at org.junit.Assert.assertEquals(Assert.java:456)
    at com.basho.riak.client.api.commands.indexes.itest.ITestRawIndexQuery.testBucketIndexHack(ITestRawIndexQuery.java:143)

[alexmoore]

@javajolt @lukebakken This one's ready too!

[lukebakken]

volatile orly?

[alexmoore]

yaRly! We set it with withNVal() a few lines down, and not in the constructor. We typically mark any non-final fields as volatile in these "core" datatype objects, to make them thread-safer with all the setup we do. I also didn't want to add two more constructors :).

[lukebakken]

So this protects against the case where two threads have access to the same YokozunaIndex object and one calls withNVal at the same time as the other calls getNVal.

[alexmoore]

Yep. Since we don't have any synchronizing .build() function, the volatile will force a synchronization of an update across cached copies of the variable in other threads. Atomic vars would be overkill here.

[lukebakken]

Just a couple comments.

[christophermancini]

👍 Tests passed on the 2nd & 3rd run. The first time, com.basho.riak.client.core.operations.itest.ITestListKeysOperation hung up on me indefinitely. It could have been my local config. Code changes look good.

[alexmoore]

@christophermancini What was the error the first time?

[christophermancini]

@alexmoore no error, just sat there for a good 10+ minutes till I killed it.

[alexmoore]

@christophermancini Ok. That's one of those tests that I have suspicions about and want to rewrite at some point so I'll note it in my log.

[christophermancini]

That was my guess, which is why I noted it but wasn't too concerned.