Skip to content

Fix resource leaks (ComplexColumn and GenericColumn)#3629

Merged
nishantmonu51 merged 5 commits intoapache:masterfrom
metamx:generic-complex-column-leaks
Nov 2, 2016
Merged

Fix resource leaks (ComplexColumn and GenericColumn)#3629
nishantmonu51 merged 5 commits intoapache:masterfrom
metamx:generic-complex-column-leaks

Conversation

@leventov
Copy link
Copy Markdown
Member

There is also DictionaryEncodedColumn, which implements Closeable, but closed only in QueryableIndexStorageAdapter, in other places it is consistently not closed. I'm not sure DictionaryEncodedColumn.close() is correct, and doesn't close some shared resources.

@drcrallen
Copy link
Copy Markdown
Contributor

This un-swallows a bunch of checks but that's probably a good thing

@drcrallen drcrallen added this to the 0.9.3 milestone Nov 1, 2016
@drcrallen
Copy link
Copy Markdown
Contributor

👍

final boolean hasNext = currRow < numRows;
if (!hasNext && !done) {
CloseQuietly.close(timestamps);
timestamps.close();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case this throws exception, we will miss closing metrics and dims, probably we can use a Closer here ?

…getRows(), in order to try to close everything even if closing some parts thew exceptions
@leventov
Copy link
Copy Markdown
Member Author

leventov commented Nov 1, 2016

@nishantmonu51 addressed your comment.

@leventov
Copy link
Copy Markdown
Member Author

leventov commented Nov 1, 2016

Related: #3637

@nishantmonu51
Copy link
Copy Markdown
Member

👍

@nishantmonu51 nishantmonu51 merged commit 4b0d6cf into apache:master Nov 2, 2016
@fjy fjy added the Bug label Nov 2, 2016
@drcrallen drcrallen deleted the generic-complex-column-leaks branch November 2, 2016 03:58
leventov added a commit to metamx/druid that referenced this pull request Nov 2, 2016
* Remove unused ComplexColumnImpl class

* Remove throws IOException from close() in GenericColumn, ComplexColumn, IndexedFloats and IndexedLongs

* Use concise try-with-resources syntax in several places

* Fix resource leaks (ComplexColumn and GenericColumn) in SegmentAnalyzer, SearchQueryRunner, QueryableIndexIndexableAdapter and QueryableIndexStorageAdapter

* Use Closer in Iterable, returned from QueryableIndexIndexableAdapter.getRows(), in order to try to close everything even if closing some parts thew exceptions

# Conflicts:
#	processing/src/main/java/io/druid/segment/QueryableIndexIndexableAdapter.java
#	processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java
#	processing/src/main/java/io/druid/segment/column/SimpleColumn.java
#	processing/src/main/java/io/druid/segment/data/BlockLayoutIndexedLongSupplier.java
leventov added a commit to metamx/druid that referenced this pull request Nov 2, 2016
* Remove unused ComplexColumnImpl class

* Remove throws IOException from close() in GenericColumn, ComplexColumn, IndexedFloats and IndexedLongs

* Use concise try-with-resources syntax in several places

* Fix resource leaks (ComplexColumn and GenericColumn) in SegmentAnalyzer, SearchQueryRunner, QueryableIndexIndexableAdapter and QueryableIndexStorageAdapter

* Use Closer in Iterable, returned from QueryableIndexIndexableAdapter.getRows(), in order to try to close everything even if closing some parts thew exceptions

# Conflicts:
#	processing/src/main/java/io/druid/segment/QueryableIndexIndexableAdapter.java
#	processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java
#	processing/src/main/java/io/druid/segment/column/SimpleColumn.java
#	processing/src/main/java/io/druid/segment/data/BlockLayoutIndexedLongSupplier.java
fjy pushed a commit that referenced this pull request Nov 3, 2016
* Remove unused ComplexColumnImpl class

* Remove throws IOException from close() in GenericColumn, ComplexColumn, IndexedFloats and IndexedLongs

* Use concise try-with-resources syntax in several places

* Fix resource leaks (ComplexColumn and GenericColumn) in SegmentAnalyzer, SearchQueryRunner, QueryableIndexIndexableAdapter and QueryableIndexStorageAdapter

* Use Closer in Iterable, returned from QueryableIndexIndexableAdapter.getRows(), in order to try to close everything even if closing some parts thew exceptions

# Conflicts:
#	processing/src/main/java/io/druid/segment/QueryableIndexIndexableAdapter.java
#	processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java
#	processing/src/main/java/io/druid/segment/column/SimpleColumn.java
#	processing/src/main/java/io/druid/segment/data/BlockLayoutIndexedLongSupplier.java
fundead pushed a commit to fundead/druid that referenced this pull request Dec 7, 2016
* Remove unused ComplexColumnImpl class

* Remove throws IOException from close() in GenericColumn, ComplexColumn, IndexedFloats and IndexedLongs

* Use concise try-with-resources syntax in several places

* Fix resource leaks (ComplexColumn and GenericColumn) in SegmentAnalyzer, SearchQueryRunner, QueryableIndexIndexableAdapter and QueryableIndexStorageAdapter

* Use Closer in Iterable, returned from QueryableIndexIndexableAdapter.getRows(), in order to try to close everything even if closing some parts thew exceptions
seoeun25 pushed a commit to seoeun25/incubator-druid that referenced this pull request Feb 25, 2022
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.

4 participants