[SPARK-23345][SQL] Remove open stream record even closing it fails #20524
Conversation
|
LGTM |
|
Test build #87147 has finished for PR 20524 at commit
|
|
retest this please |
|
Test build #87150 has finished for PR 20524 at commit
|
|
retest this please |
|
retest this please. |
| @@ -111,7 +111,7 @@ trait SharedSparkSession | |||
| spark.sharedState.cacheManager.clearCache() | |||
| // files can be closed from other threads, so wait a bit | |||
| // normally this doesn't take more than 1s | |||
| eventually(timeout(10.seconds)) { | |||
| eventually(timeout(10.seconds), interval(2.seconds)) { | |||
There was a problem hiding this comment.
what's the problem if interval is small?
There was a problem hiding this comment.
assertNoOpenStreams and removeOpenStream are both synchronized on openStreams map. I want to increase interval to prevent starvation for removeOpenStream.
There was a problem hiding this comment.
The default is 15 milliseconds. It should be fine.
But, I am also fine to change this to 2 seconds. I checked the test history and this does not increase the testing time.
|
Test build #87155 has finished for PR 20524 at commit
|
|
Test build #87156 has finished for PR 20524 at commit
|
|
Test build #87157 has finished for PR 20524 at commit
|
## What changes were proposed in this pull request? When `DebugFilesystem` closes opened stream, if any exception occurs, we still need to remove the open stream record from `DebugFilesystem`. Otherwise, it goes to report leaked filesystem connection. ## How was this patch tested? Existing tests. Author: Liang-Chi Hsieh <viirya@gmail.com> Closes #20524 from viirya/SPARK-23345. (cherry picked from commit 9841ae0) Signed-off-by: gatorsmile <gatorsmile@gmail.com>
|
LGTM Thanks! Merged to master/2.3 |
## What changes were proposed in this pull request? When `DebugFilesystem` closes opened stream, if any exception occurs, we still need to remove the open stream record from `DebugFilesystem`. Otherwise, it goes to report leaked filesystem connection. ## How was this patch tested? Existing tests. Author: Liang-Chi Hsieh <viirya@gmail.com> Closes apache#20524 from viirya/SPARK-23345.
What changes were proposed in this pull request?
When
DebugFilesystemcloses opened stream, if any exception occurs, we still need to remove the open stream record fromDebugFilesystem. Otherwise, it goes to report leaked filesystem connection.How was this patch tested?
Existing tests.