Refactoring of IncrementalIndex

[navis]

Current on-heap incremental indexer maps key to integer and maps the integer again to aggregators. Directly mapping key to aggregators would make things simple.

[fjy]

let's just delete this whole file

[navis]

can I? ok

[fjy]

can we add a log.error()?

[fjy]

Retracting +1 for now pending further review

[navis]

@fjy Ah, sorry. I've missed your comment.

[navis]

Seemed one of the transient errors.

io.druid.curator.announcement.AnnouncerTest
testSessionKilled(io.druid.curator.announcement.AnnouncerTest)  Time elapsed: 2.333 sec  <<< FAILURE!
java.lang.AssertionError: expected null, but was:<11,11,1450659858789,1450659858789,0,0,0,95070444354404353,25,0,11
>
    at org.junit.Assert.fail(Assert.java:88)
    at org.junit.Assert.failNotNull(Assert.java:664)
    at org.junit.Assert.assertNull(Assert.java:646)
    at org.junit.Assert.assertNull(Assert.java:656)
    at io.druid.curator.announcement.AnnouncerTest.testSessionKilled(AnnouncerTest.java:176)

[nishantmonu51]

Retracting +1, since there are many changes since i last looked into it, need to review again

[navis]

@nishantmonu51 Squashed!

[fjy]

@himanshug @xvrl do u guys want to take a look at this? I think it is ready. Will merge tomm if no more comments.

[gianm]

I don't think "transient" is necessary, this object is not going to be serialized

[navis]

I use it as a kind of annotation which means this field is transient sometimes. I'll remove it.

[xvrl]

@fjy I think this needs some more work, I have concerns in terms of additional GC pressure / performance. Some of the code that prevented race conditions we've observed in the past also seems to have been removed without explanation.

[navis]

@xvrl As you said it's broken, sorry. I've reverted the logic. Thanks.

[navis]

rebased on trunk

[navis]

rebased

[fjy]

@xvrl can you provide more details of what you'd like to see in this PR? Be specific otherwise it is hard for @navis to know what to do

[himanshug]

@navis this PR makes the assumption that IncrementalIndex only works with on heap Aggregators .. I'm implementing a OffheapIncrementalIndex in #2325 .

[navis]

@himanshug Rebased on master and revived some tests for off heap indexer.

[fjy]

@cheddar can you help review this?

[fjy]

Removing milestone for now as I think this will require more thought

[fjy]

@navis have you had a chance to review @himanshug's comment?

[navis]

@fjy Yep, I've done it but this patch seemed not necessary (and hard to maintain). Let's close this.