Skip to content

fix reference counting for segments#2301

Merged
fjy merged 2 commits intoapache:masterfrom
metamx:fix-2299_2
Jan 20, 2016
Merged

fix reference counting for segments#2301
fjy merged 2 commits intoapache:masterfrom
metamx:fix-2299_2

Conversation

@nishantmonu51
Copy link
Copy Markdown
Member

This PR has two changes -

  1. fixes ReferenceCountingQueryRunner - It now reports missing segment when query is already closed
  2. fixes Queries can still access IncrementalIndex even after its closed.  #2299 by adding a swapLock to Firehydrant to make sure incrementalIndex is not swapped and closed between getSegment and increment is call.

Also removed unnecessary checks and ReportMissingSegment for null adapters.

@nishantmonu51
Copy link
Copy Markdown
Member Author

AnnouncerTest failed by -
org.apache.zookeeper.KeeperException$NoNodeException: KeeperErrorCode = NoNode
https://travis-ci.org/druid-io/druid/jobs/103585051

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jan 20, 2016

👍

what do you think about also deleting RealtimePlumber.java 330:335 (block starting here: https://github.com/druid-io/druid/pull/2301/files#diff-e7c5ce8efa97c42949b01290c2bb4a47L330)?

ReportTimlineMissing for hydrants is bad; I think that code won't get called anyway (hydrants in sinks shouldn't be null, and segments in a hydrant shouldn't be null) so should be okay to delete.

@drcrallen
Copy link
Copy Markdown
Contributor

This is a bug against master

sink never have null hydrants and hydrants never have null adapters
@nishantmonu51
Copy link
Copy Markdown
Member Author

@gianm: deleted the dead code block.

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jan 20, 2016

👍

@drcrallen drcrallen added this to the 0.9.0 milestone Jan 20, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 20, 2016

@nishantmonu51 can we file an issue for the announcer test failing?

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 20, 2016

👍

fjy added a commit that referenced this pull request Jan 20, 2016
fix reference counting for segments
@fjy fjy merged commit 0c5f4b9 into apache:master Jan 20, 2016
@drcrallen drcrallen deleted the fix-2299_2 branch January 20, 2016 19:27
@xvrl
Copy link
Copy Markdown
Member

xvrl commented Jan 20, 2016

@nishantmonu51 any way we can add tests for those?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Queries can still access IncrementalIndex even after its closed.

5 participants