[GitHub] [incubator-livy] asfgit closed pull request #163: [LIVY-513][LIVY-517][TEST] Fix SessionHeartbeatSpec flakiness, occasional NPE
asfgit closed pull request #163: [LIVY-513][LIVY-517][TEST] Fix SessionHeartbeatSpec flakiness, occasional NPE URL: https://github.com/apache/incubator-livy/pull/163 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] vanzin commented on issue #163: [LIVY-513][LIVY-517][TEST] Fix SessionHeartbeatSpec flakiness, occasional NPE
vanzin commented on issue #163: [LIVY-513][LIVY-517][TEST] Fix SessionHeartbeatSpec flakiness, occasional NPE URL: https://github.com/apache/incubator-livy/pull/163#issuecomment-476915265 Merging to master. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] asfgit closed pull request #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer
asfgit closed pull request #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer URL: https://github.com/apache/incubator-livy/pull/162 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] vanzin commented on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer
vanzin commented on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer URL: https://github.com/apache/incubator-livy/pull/162#issuecomment-476913608 Merging to master. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] codecov-io edited a comment on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer
codecov-io edited a comment on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer URL: https://github.com/apache/incubator-livy/pull/162#issuecomment-476429537 # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=h1) Report > Merging [#162](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/07d216d33a5420004262c9b8d2acc2751547cc96?src=pr=desc) will **increase** coverage by `0.14%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=tree) ```diff @@ Coverage Diff @@ ## master #162 +/- ## + Coverage 68.59% 68.73% +0.14% + Complexity 905 904 -1 Files 100 100 Lines 5662 5662 Branches848 848 + Hits 3884 3892 +8 + Misses 1225 1216 -9 - Partials553 554 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ain/java/org/apache/livy/rsc/driver/RSCDriver.java](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvUlNDRHJpdmVyLmphdmE=) | `77.96% <0%> (-0.85%)` | `41% <0%> (-1%)` | | | [.../scala/org/apache/livy/sessions/SessionState.scala](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-Y29yZS9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvc2Vzc2lvbnMvU2Vzc2lvblN0YXRlLnNjYWxh) | `61.11% <0%> (+2.77%)` | `2% <0%> (ø)` | :arrow_down: | | [...ain/scala/org/apache/livy/utils/SparkYarnApp.scala](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS91dGlscy9TcGFya1lhcm5BcHAuc2NhbGE=) | `79.57% <0%> (+6.33%)` | `33% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=footer). Last update [07d216d...0c8400f](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] codecov-io edited a comment on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer
codecov-io edited a comment on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer URL: https://github.com/apache/incubator-livy/pull/162#issuecomment-476429537 # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=h1) Report > Merging [#162](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/07d216d33a5420004262c9b8d2acc2751547cc96?src=pr=desc) will **increase** coverage by `0.14%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=tree) ```diff @@ Coverage Diff @@ ## master #162 +/- ## + Coverage 68.59% 68.73% +0.14% + Complexity 905 904 -1 Files 100 100 Lines 5662 5662 Branches848 848 + Hits 3884 3892 +8 + Misses 1225 1216 -9 - Partials553 554 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ain/java/org/apache/livy/rsc/driver/RSCDriver.java](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvUlNDRHJpdmVyLmphdmE=) | `77.96% <0%> (-0.85%)` | `41% <0%> (-1%)` | | | [.../scala/org/apache/livy/sessions/SessionState.scala](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-Y29yZS9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvc2Vzc2lvbnMvU2Vzc2lvblN0YXRlLnNjYWxh) | `61.11% <0%> (+2.77%)` | `2% <0%> (ø)` | :arrow_down: | | [...ain/scala/org/apache/livy/utils/SparkYarnApp.scala](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS91dGlscy9TcGFya1lhcm5BcHAuc2NhbGE=) | `79.57% <0%> (+6.33%)` | `33% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=footer). Last update [07d216d...0c8400f](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] codecov-io edited a comment on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer
codecov-io edited a comment on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer URL: https://github.com/apache/incubator-livy/pull/162#issuecomment-476429537 # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=h1) Report > Merging [#162](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/07d216d33a5420004262c9b8d2acc2751547cc96?src=pr=desc) will **increase** coverage by `0.14%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=tree) ```diff @@ Coverage Diff @@ ## master #162 +/- ## + Coverage 68.59% 68.73% +0.14% + Complexity 905 904 -1 Files 100 100 Lines 5662 5662 Branches848 848 + Hits 3884 3892 +8 + Misses 1225 1216 -9 - Partials553 554 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ain/java/org/apache/livy/rsc/driver/RSCDriver.java](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvUlNDRHJpdmVyLmphdmE=) | `77.96% <0%> (-0.85%)` | `41% <0%> (-1%)` | | | [.../scala/org/apache/livy/sessions/SessionState.scala](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-Y29yZS9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvc2Vzc2lvbnMvU2Vzc2lvblN0YXRlLnNjYWxh) | `61.11% <0%> (+2.77%)` | `2% <0%> (ø)` | :arrow_down: | | [...ain/scala/org/apache/livy/utils/SparkYarnApp.scala](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS91dGlscy9TcGFya1lhcm5BcHAuc2NhbGE=) | `79.57% <0%> (+6.33%)` | `33% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=footer). Last update [07d216d...0c8400f](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] codecov-io edited a comment on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer
codecov-io edited a comment on issue #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer URL: https://github.com/apache/incubator-livy/pull/162#issuecomment-476429537 # [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=h1) Report > Merging [#162](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-livy/commit/07d216d33a5420004262c9b8d2acc2751547cc96?src=pr=desc) will **increase** coverage by `0.14%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=tree) ```diff @@ Coverage Diff @@ ## master #162 +/- ## + Coverage 68.59% 68.73% +0.14% + Complexity 905 904 -1 Files 100 100 Lines 5662 5662 Branches848 848 + Hits 3884 3892 +8 + Misses 1225 1216 -9 - Partials553 554 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ain/java/org/apache/livy/rsc/driver/RSCDriver.java](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-cnNjL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9saXZ5L3JzYy9kcml2ZXIvUlNDRHJpdmVyLmphdmE=) | `77.96% <0%> (-0.85%)` | `41% <0%> (-1%)` | | | [.../scala/org/apache/livy/sessions/SessionState.scala](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-Y29yZS9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2xpdnkvc2Vzc2lvbnMvU2Vzc2lvblN0YXRlLnNjYWxh) | `61.11% <0%> (+2.77%)` | `2% <0%> (ø)` | :arrow_down: | | [...ain/scala/org/apache/livy/utils/SparkYarnApp.scala](https://codecov.io/gh/apache/incubator-livy/pull/162/diff?src=pr=tree#diff-c2VydmVyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvbGl2eS91dGlscy9TcGFya1lhcm5BcHAuc2NhbGE=) | `79.57% <0%> (+6.33%)` | `33% <0%> (ø)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=footer). Last update [07d216d...0c8400f](https://codecov.io/gh/apache/incubator-livy/pull/162?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-livy] gyogal opened a new pull request #163: [LIVY-513][LIVY-517][TEST] Fix SessionHeartbeatSpec flakiness, occasional NPE
gyogal opened a new pull request #163: [LIVY-513][LIVY-517][TEST] Fix SessionHeartbeatSpec flakiness, occasional NPE URL: https://github.com/apache/incubator-livy/pull/163 ## What changes were proposed in this pull request? This PR aims to fix the following issues in the test suite `SessionHeartbeatSpec`: * Since there is no implementation provided for `stop()` in `mock[TestSession]`, Mockito returns `null` by default and this causes `SessionManager#delete` to throw `NullPointerException` when executing `map()` on the `Future[Unit]` object it expects. This can be observed in the unit test output (`server/target/unit-tests.log`): ``` 19/03/25 18:20:30.123 ScalaTest-main-running-SessionHeartbeatSpec INFO SessionHeartbeatSpec$$anonfun$2$TestWatchdog$1: Registering new session 0 19/03/25 18:20:30.123 ScalaTest-main-running-SessionHeartbeatSpec INFO SessionHeartbeatSpec$$anonfun$2$TestWatchdog$1: Registering new session 1 19/03/25 18:20:30.124 ScalaTest-main-running-SessionHeartbeatSpec INFO SessionHeartbeatSpec$$anonfun$2$TestWatchdog$1: Session 0 expired. Last heartbeat is at null. 19/03/25 18:20:30.124 ScalaTest-main-running-SessionHeartbeatSpec WARN SessionHeartbeatSpec$$anonfun$2$TestWatchdog$1: Exception was thrown when deleting expired session 0 java.lang.NullPointerException at org.apache.livy.sessions.SessionManager.delete(SessionManager.scala:123) at org.apache.livy.server.interactive.SessionHeartbeatWatchdog$$anonfun$deleteExpiredSessions$1.apply(SessionHeartbeat.scala:113) at org.apache.livy.server.interactive.SessionHeartbeatWatchdog$$anonfun$deleteExpiredSessions$1.apply(SessionHeartbeat.scala:110) at scala.collection.Iterator$class.foreach(Iterator.scala:891) at scala.collection.AbstractIterator.foreach(Iterator.scala:1334) at scala.collection.MapLike$DefaultValuesIterable.foreach(MapLike.scala:206) at org.apache.livy.server.interactive.SessionHeartbeatWatchdog$class.deleteExpiredSessions(SessionHeartbeat.scala:110) at org.apache.livy.server.interactive.SessionHeartbeatSpec$$anonfun$2$TestWatchdog$1.deleteExpiredSessions(SessionHeartbeatSpec.scala:60) ``` * In some rare cases, the SessionManager GC thread may run `collectGarbage()` after the test sessions are registered but before the test is finished. The GC thread will also attempt to delete the `mock[TestSession]` objects because the mocked session's `lastActivity()` function returns 0 by default which means`currentTime - session.lastActivity > sessionTimeout` will always be true. This causes a race condition that can have the following outcomes: 1. The `stop()` method may be called twice (both by `SessionHeartbeatWatchdog#deleteExpiredSessions` and `SessionManager#collectGarbage`), resulting in a `TooManyActualInvocations` exception (LIVY-513): ``` org.mockito.exceptions.verification.TooManyActualInvocations: testSession$1.stop(); Wanted 1 time: -> at org.apache.livy.server.interactive.SessionHeartbeatSpec$$anonfun$2$$anonfun$apply$mcV$sp$5.apply$mcV$sp(SessionHeartbeatSpec.scala:93) But was 2 times. Undesired invocation: -> at org.apache.livy.sessions.SessionManager.delete(SessionManager.scala:124) at org.apache.livy.server.interactive.SessionHeartbeatSpec$$anonfun$2$$anonfun$apply$mcV$sp$5.apply$mcV$sp(SessionHeartbeatSpec.scala:93) at org.apache.livy.server.interactive.SessionHeartbeatSpec$$anonfun$2$$anonfun$apply$mcV$sp$5.apply(SessionHeartbeatSpec.scala:70) at org.apache.livy.server.interactive.SessionHeartbeatSpec$$anonfun$2$$anonfun$apply$mcV$sp$5.apply(SessionHeartbeatSpec.scala:70) at org.scalatest.Transformer$$anonfun$apply$1.apply$mcV$sp(Transformer.scala:22) at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85) at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104) at org.scalatest.Transformer.apply(Transformer.scala:22) at org.scalatest.Transformer.apply(Transformer.scala:20) at org.scalatest.FunSpecLike$$anon$1.apply(FunSpecLike.scala:422) at org.scalatest.Suite$class.withFixture(Suite.scala:1122) ... ``` 2. The `stop()` method may be called by `collectGarbage()` after `deleteExpiredSessions()`, but before the test is finished, which causes the following error message to appear on the console (LIVY-517). ``` Deleting Mock for TestSession$1, hashCode: 138753 because it was inactive for more than 360.0 ms. Exception in thread "session gc thread" java.lang.NullPointerException at
[GitHub] [incubator-livy] vanzin commented on a change in pull request #162: [LIVY-572] Avoid usage of spark classes in ColumnBuffer
vanzin commented on a change in pull request #162: [LIVY-572] Avoid usage of
spark classes in ColumnBuffer
URL: https://github.com/apache/incubator-livy/pull/162#discussion_r269165870
##
File path:
thriftserver/session/src/main/java/org/apache/livy/thriftserver/session/ResultSet.java
##
@@ -43,7 +63,13 @@ public void addRow(Object[] fields) {
}
for (int i = 0; i < fields.length; i++) {
- columns[i].add(fields[i]);
+ Object value;
+ if (columnIsString[i]) {
Review comment:
Using `==` there's actually no more overhead at all. In fact, if you look at
CPU caches and things like that it's probably faster since there's less risk of
a cache miss...
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
