From c3f5fe3f68e86af1bab37a291bfeb9b51b9957a3 Mon Sep 17 00:00:00 2001 From: edvardas Date: Tue, 9 Jan 2024 12:32:41 +0200 Subject: [PATCH 1/5] Save DEAD state if Yarn client throws exception --- .../lighter/backend/yarn/YarnBackend.java | 5 +++-- .../lighter/backend/yarn/YarnBackendTest.groovy | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java b/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java index 08826cff..c69bd879 100644 --- a/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java +++ b/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java @@ -49,7 +49,8 @@ public YarnBackend(YarnProperties yarnProperties, YarnClient client, AppConfigur @Override public Optional getInfo(Application application) { return getYarnApplicationId(application) - .flatMap(id -> getState(id).map(state -> new ApplicationInfo(state, id))); + .flatMap(id -> getState(id).map(state -> new ApplicationInfo(state, id))) + .or(() -> Optional.of(new ApplicationInfo(ApplicationState.DEAD, null))); } private Optional getState(String id) { @@ -145,7 +146,7 @@ private Optional getYarnApplicationId(Application application) { .map(ApplicationId::toString); } catch (YarnException | IOException e) { LOG.error("Failed to get app id for app: {}", application, e); - throw new IllegalStateException(e); + return Optional.empty(); } }); } diff --git a/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy b/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy index 349459d1..36cf7f5b 100644 --- a/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy +++ b/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy @@ -49,6 +49,19 @@ class YarnBackendTest extends Specification { info.state == ApplicationState.BUSY } + def "gets app info with DEAD state if IOException thrown"() { + given: + def app = newApplication(null) + + when: + def info = backend.getInfo(app).get() + + then: + 1 * client.getApplications(*_) >> { throw new IOException() } + info.getApplicationId() == null + info.getState() == ApplicationState.DEAD + } + def "fetches logs"() { given: @@ -104,4 +117,5 @@ class YarnBackendTest extends Specification { client.getApplications(*_) >> [yarnApp] client.getApplicationReport(yarnId) >> yarnApp } + } From 517ac12f8a1c4d0d09ee89f74aca4dff198ff8af Mon Sep 17 00:00:00 2001 From: edvardas Date: Wed, 10 Jan 2024 09:54:54 +0200 Subject: [PATCH 2/5] Return optional empty if exception thrown --- .../com/exacaster/lighter/backend/yarn/YarnBackend.java | 5 ++--- .../exacaster/lighter/backend/yarn/YarnBackendTest.groovy | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java b/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java index c69bd879..3d0f9c98 100644 --- a/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java +++ b/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java @@ -49,8 +49,7 @@ public YarnBackend(YarnProperties yarnProperties, YarnClient client, AppConfigur @Override public Optional getInfo(Application application) { return getYarnApplicationId(application) - .flatMap(id -> getState(id).map(state -> new ApplicationInfo(state, id))) - .or(() -> Optional.of(new ApplicationInfo(ApplicationState.DEAD, null))); + .flatMap(id -> getState(id).map(state -> new ApplicationInfo(state, id))); } private Optional getState(String id) { @@ -144,7 +143,7 @@ private Optional getYarnApplicationId(Application application) { .max(Comparator.comparing(ApplicationReport::getStartTime)) .map(ApplicationReport::getApplicationId) .map(ApplicationId::toString); - } catch (YarnException | IOException e) { + } catch (YarnException | IOException | RuntimeException e) { LOG.error("Failed to get app id for app: {}", application, e); return Optional.empty(); } diff --git a/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy b/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy index 36cf7f5b..794f946a 100644 --- a/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy +++ b/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy @@ -49,17 +49,16 @@ class YarnBackendTest extends Specification { info.state == ApplicationState.BUSY } - def "gets app info with DEAD state if IOException thrown"() { + def "gets empty app info if IOException thrown"() { given: def app = newApplication(null) when: - def info = backend.getInfo(app).get() + def info = backend.getInfo(app) then: 1 * client.getApplications(*_) >> { throw new IOException() } - info.getApplicationId() == null - info.getState() == ApplicationState.DEAD + info.isEmpty() } From 250fbe72f9c3889074bc386e088db9c7cefeb25e Mon Sep 17 00:00:00 2001 From: minutis Date: Wed, 10 Jan 2024 16:53:07 +0200 Subject: [PATCH 3/5] Return optional empty if exception thrown Add specific case for wrapped IOException --- .../exacaster/lighter/backend/yarn/YarnBackend.java | 11 ++++++++++- .../lighter/backend/yarn/YarnBackendTest.groovy | 11 +++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java b/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java index 3d0f9c98..2e7af70c 100644 --- a/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java +++ b/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java @@ -143,9 +143,18 @@ private Optional getYarnApplicationId(Application application) { .max(Comparator.comparing(ApplicationReport::getStartTime)) .map(ApplicationReport::getApplicationId) .map(ApplicationId::toString); - } catch (YarnException | IOException | RuntimeException e) { + } catch (YarnException | IOException e) { LOG.error("Failed to get app id for app: {}", application, e); return Optional.empty(); + } catch (RuntimeException e) { + // Yarn client sometimes throws IOException wrapped in RuntimeException + LOG.error("Failed to get app id for app: {}", application, e); + String message = e.getMessage(); + if (message != null && message.contains("java.io.IOException")) { + return Optional.empty(); + } else { + throw e; + } } }); } diff --git a/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy b/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy index 794f946a..8571f892 100644 --- a/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy +++ b/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy @@ -61,6 +61,17 @@ class YarnBackendTest extends Specification { info.isEmpty() } + def "gets empty app info if RuntimeException with IOException thrown"() { + given: + def app = newApplication(null) + + when: + def info = backend.getInfo(app) + + then: + 1 * client.getApplications(*_) >> { throw new RuntimeException("java.lang.RuntimeException: Exception in thread \"main\" java.io.IOException: Could not get block locations") } + info.isEmpty() + } def "fetches logs"() { given: From 702c3392212212aa3abbb12a331c428ec26b9966 Mon Sep 17 00:00:00 2001 From: minutis Date: Wed, 10 Jan 2024 21:50:08 +0200 Subject: [PATCH 4/5] Return optional empty if exception thrown Add specific case for wrapped IOException --- .../com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy b/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy index 8571f892..5fb30dc5 100644 --- a/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy +++ b/server/src/test/groovy/com/exacaster/lighter/backend/yarn/YarnBackendTest.groovy @@ -69,7 +69,7 @@ class YarnBackendTest extends Specification { def info = backend.getInfo(app) then: - 1 * client.getApplications(*_) >> { throw new RuntimeException("java.lang.RuntimeException: Exception in thread \"main\" java.io.IOException: Could not get block locations") } + 1 * client.getApplications(*_) >> { throw new RuntimeException(new IOException("test")) } info.isEmpty() } From a111da902eb643ed8b10e56bade613b901b8b524 Mon Sep 17 00:00:00 2001 From: minutis Date: Thu, 11 Jan 2024 09:58:13 +0200 Subject: [PATCH 5/5] Return optional empty if exception thrown Add specific case for wrapped IOException --- .../com/exacaster/lighter/backend/yarn/YarnBackend.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java b/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java index 2e7af70c..650d252a 100644 --- a/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java +++ b/server/src/main/java/com/exacaster/lighter/backend/yarn/YarnBackend.java @@ -149,12 +149,11 @@ private Optional getYarnApplicationId(Application application) { } catch (RuntimeException e) { // Yarn client sometimes throws IOException wrapped in RuntimeException LOG.error("Failed to get app id for app: {}", application, e); - String message = e.getMessage(); - if (message != null && message.contains("java.io.IOException")) { + if (e.getCause() instanceof IOException) { return Optional.empty(); - } else { - throw e; } + + throw e; } }); }