Skip to content

Make OnHeapIncrementalIndex clean maps on close()#2197

Merged
fjy merged 1 commit intoapache:masterfrom
metamx:clearIncIndexClose
Jan 7, 2016
Merged

Make OnHeapIncrementalIndex clean maps on close()#2197
fjy merged 1 commit intoapache:masterfrom
metamx:clearIncIndexClose

Conversation

@drcrallen
Copy link
Copy Markdown
Contributor

Related to #2149 this PR tries to help make sure any accidental references to an incremental index after its close() method is called has minimal memory impact.

@drcrallen
Copy link
Copy Markdown
Contributor Author

ServiceAnnouncerTest.testServiceAnnouncement:55->createAndAnnounceServices:102 » ConnectionLoss

@drcrallen drcrallen closed this Jan 4, 2016
@drcrallen drcrallen reopened this Jan 4, 2016
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jan 4, 2016

@drcrallen why did this come up? It seems like the real problem is that something is keeping around indexes that it shouldn't be keeping around…

@drcrallen
Copy link
Copy Markdown
Contributor Author

@gianm #2149 (comment)

Mostly because it is easy to screw up keeping indexes around. I had a similar problem in the spark batch indexer because I was handling the reference to the incremental index in an inefficient manner. It is a very easy mistake to make, and this change is intended to minimize its impact. Though yes, you are correct, having the incremental index object itself garbage collected properly is the preferred route.

@nishantmonu51
Copy link
Copy Markdown
Member

👍,Looks good to me. this can be merged unless @gianm has any more comments ?

@binlijin
Copy link
Copy Markdown
Contributor

binlijin commented Jan 6, 2016

+1

@navis
Copy link
Copy Markdown
Contributor

navis commented Jan 6, 2016

IncrementalIndex also has some data structures to be cleaned up on close. But that's all tiny and can be handled in #2169 after this is merged. +1 for me.

fjy added a commit that referenced this pull request Jan 7, 2016
Make OnHeapIncrementalIndex clean maps on close()
@fjy fjy merged commit d0b10c2 into apache:master Jan 7, 2016
nishantmonu51 added a commit to metamx/druid that referenced this pull request Jan 20, 2016
@fjy fjy modified the milestone: 0.9.0 Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants