HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-642306798
Looks fine in general - one thing looks to be missed is,
`LevelDBIterator.close()` should also call `db.closeIterator(this);` to make
sure tracker can indicate it.
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641702431
So looks like there're two different issues - 1) resource leaks in general
2) concurrent usage on KV store.
@srowen and me have been talking about the issue
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641700315
Oh OK. Sorry I missed. At least we close it properly when calling
`closeableIterator()`, but still in question for using KVStoreView directly. If
we are confident on
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641676010
> Lots of the paths actually do close the iterator explicitly. And I'm still
not sure how .toSeq doesn't consume it.
I'm not sure you're referring KV store
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641676010
> Lots of the paths actually do close the iterator explicitly. And I'm still
not sure how .toSeq doesn't consume it.
I'm not sure you're referring KV store
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641676010
> Lots of the paths actually do close the iterator explicitly. And I'm still
not sure how .toSeq doesn't consume it.
I'm not sure you're referring KV store
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641672674
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641668567
IMHO, KVStoreView shouldn't implement `Iterable` directly - this leads
callers to simply call `iterator` or wrap with `asScala` and completely forget
about the
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641668567
IMHO, KVStoreView shouldn't implement `Iterable` directly - this leads
callers to simply call `iterator` or wrap with `asScala` and completely forget
about the
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641652906
Oh I was pointing out innocent one. `KVStoreView` is the culprit, though the
class doc describes the warning.
HeartSaVioR edited a comment on pull request #28769:
URL: https://github.com/apache/spark/pull/28769#issuecomment-641649819
I guess the interface matters - in-memory KV store doesn't need to have
close in its iterator of course (it has, but no-op), but level DB KV store
should. The code
11 matches
Mail list logo
