Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id BBE3D200CE1 for ; Thu, 31 Aug 2017 23:51:03 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id BA27216C071; Thu, 31 Aug 2017 21:51:03 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id B367F16C070 for ; Thu, 31 Aug 2017 23:51:02 +0200 (CEST) Received: (qmail 82394 invoked by uid 500); 31 Aug 2017 21:51:01 -0000 Mailing-List: contact notifications-help@asterixdb.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@asterixdb.apache.org Delivered-To: mailing list notifications@asterixdb.apache.org Received: (qmail 82385 invoked by uid 99); 31 Aug 2017 21:51:01 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 31 Aug 2017 21:51:01 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 7D01ACF790 for ; Thu, 31 Aug 2017 21:51:00 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.126 X-Spam-Level: ** X-Spam-Status: No, score=2.126 tagged_above=-999 required=6.31 tests=[MISSING_HEADERS=1.207, SPF_FAIL=0.919] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id vxzKPgHfHiJk for ; Thu, 31 Aug 2017 21:50:59 +0000 (UTC) Received: from vitalstatistix.ics.uci.edu (vitalstatistix.ics.uci.edu [128.195.52.38]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTP id E02935FC21 for ; Thu, 31 Aug 2017 21:50:58 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by vitalstatistix.ics.uci.edu (Postfix) with ESMTP id 469BC100821; Thu, 31 Aug 2017 14:50:58 -0700 (PDT) Date: Thu, 31 Aug 2017 14:50:58 -0700 From: "abdullah alamoudi (Code Review)" CC: Jenkins , Murtadha Hubail , Michael Blow Reply-To: bamousaa@gmail.com X-Gerrit-MessageType: merged Subject: Change in asterixdb[master]: [NO ISSUE][ING] Caller checks for active jobs failures X-Gerrit-Change-Id: I85146028be70f4631d1ef2696489a4624bf23ad4 X-Gerrit-ChangeURL: X-Gerrit-Commit: 10cc52d9228b341a1dba898d267f2526123de6fc In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/2.12.7 Message-Id: <20170831215058.469BC100821@vitalstatistix.ics.uci.edu> archived-at: Thu, 31 Aug 2017 21:51:03 -0000 abdullah alamoudi has submitted this change and it was merged. Change subject: [NO ISSUE][ING] Caller checks for active jobs failures ...................................................................... [NO ISSUE][ING] Caller checks for active jobs failures - user model changes: no - storage format changes: no - interface changes: yes -IActiveEntityEventSubscriber.sync() does not throw failure anymore and caller is responsible for checking for failures. details: - Previously, certain kinds of failures are sometimes thrown and sometimes not when syncing a subscriber. This lead to confusions and bugs. After this change, the caller to sync is responsible for checking for failures. Change-Id: I85146028be70f4631d1ef2696489a4624bf23ad4 Reviewed-on: https://asterix-gerrit.ics.uci.edu/1986 Sonar-Qube: Jenkins Tested-by: Jenkins Contrib: Jenkins Integration-Tests: Jenkins Reviewed-by: Murtadha Hubail --- M asterixdb/asterix-active/src/main/java/org/apache/asterix/active/IActiveEntityEventSubscriber.java M asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/ActiveEntityEventsListener.java M asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/FeedEventsListener.java M asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/ActiveEventsListenerTest.java M asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/DummyFeedEventsListener.java M asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/TestEventsListener.java M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/AbstractSubscriber.java 7 files changed, 20 insertions(+), 13 deletions(-) Approvals: Anon. E. Moose #1000171: Jenkins: Verified; No violations found; ; Verified Murtadha Hubail: Looks good to me, approved diff --git a/asterixdb/asterix-active/src/main/java/org/apache/asterix/active/IActiveEntityEventSubscriber.java b/asterixdb/asterix-active/src/main/java/org/apache/asterix/active/IActiveEntityEventSubscriber.java index f9357b4..142e84b 100644 --- a/asterixdb/asterix-active/src/main/java/org/apache/asterix/active/IActiveEntityEventSubscriber.java +++ b/asterixdb/asterix-active/src/main/java/org/apache/asterix/active/IActiveEntityEventSubscriber.java @@ -43,9 +43,9 @@ /** * Wait until the terminal event has been received * - * @throws Exception + * @throws InterruptedException */ - void sync() throws HyracksDataException, InterruptedException; + void sync() throws InterruptedException; /** * callback upon successful subscription diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/ActiveEntityEventsListener.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/ActiveEntityEventsListener.java index 3b4b974..368c2da 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/ActiveEntityEventsListener.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/ActiveEntityEventsListener.java @@ -507,8 +507,13 @@ .submit(() -> rt.resumeOrRecover(metadataProvider)); try { subscriber.sync(); - } catch (Exception e) { + if (subscriber.getFailure() != null) { + LOGGER.log(Level.WARNING, "Failure while attempting to resume " + entityId, + subscriber.getFailure()); + } + } catch (InterruptedException e) { LOGGER.log(Level.WARNING, "Failure while attempting to resume " + entityId, e); + Thread.currentThread().interrupt(); throw HyracksDataException.create(e); } } finally { diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/FeedEventsListener.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/FeedEventsListener.java index 124e56e..b710f2b 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/FeedEventsListener.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/active/FeedEventsListener.java @@ -90,7 +90,7 @@ ((QueryTranslator) statementExecutor).getSessionOutput(), mdProvider, feed, feedConnections, compilationProvider, storageComponentProvider, statementExecutorFactory, hcc); JobSpecification feedJob = jobInfo.getLeft(); - IActiveEntityEventSubscriber eventSubscriber = + WaitForStateSubscriber eventSubscriber = new WaitForStateSubscriber(this, Collections.singleton(ActivityState.RUNNING)); feedJob.setProperty(ActiveNotificationHandler.ACTIVE_ENTITY_PROPERTY_NAME, entityId); // TODO(Yingyi): currently we do not check IFrameWriter protocol violations for Feed jobs. @@ -99,6 +99,9 @@ boolean wait = Boolean.parseBoolean(mdProvider.getConfig().get(StartFeedStatement.WAIT_FOR_COMPLETION)); JobUtils.runJob(hcc, feedJob, false); eventSubscriber.sync(); + if (eventSubscriber.getFailure() != null) { + throw eventSubscriber.getFailure(); + } if (wait) { IActiveEntityEventSubscriber stoppedSubscriber = new WaitForStateSubscriber(this, EnumSet.of(ActivityState.STOPPED, ActivityState.PERMANENTLY_FAILED)); diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/ActiveEventsListenerTest.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/ActiveEventsListenerTest.java index 97283b6..e03ee6e 100644 --- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/ActiveEventsListenerTest.java +++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/ActiveEventsListenerTest.java @@ -753,8 +753,8 @@ listener.onStart(Behavior.FAIL_COMPILE); WaitForStateSubscriber tempFailSubscriber = new WaitForStateSubscriber(listener, EnumSet.of(ActivityState.TEMPORARILY_FAILED)); - clusterController.jobFinish(listener.getJobId(), JobStatus.FAILURE, - Collections.singletonList(new HyracksDataException("Runtime Failure"))); + List exceptions = Collections.singletonList(new HyracksDataException("Runtime Failure")); + clusterController.jobFinish(listener.getJobId(), JobStatus.FAILURE, exceptions); // recovery is ongoing listener.onStart(Behavior.STEP_SUCCEED); tempFailSubscriber.sync(); diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/DummyFeedEventsListener.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/DummyFeedEventsListener.java index 961b731..4de5813 100644 --- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/DummyFeedEventsListener.java +++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/DummyFeedEventsListener.java @@ -51,10 +51,13 @@ @Override protected void doStart(MetadataProvider metadataProvider) throws HyracksDataException, AlgebricksException { - IActiveEntityEventSubscriber eventSubscriber = + WaitForStateSubscriber eventSubscriber = new WaitForStateSubscriber(this, Collections.singleton(ActivityState.RUNNING)); try { eventSubscriber.sync(); + if (eventSubscriber.getFailure() != null) { + throw eventSubscriber.getFailure(); + } } catch (Exception e) { throw HyracksDataException.create(e); } diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/TestEventsListener.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/TestEventsListener.java index 905df72..c8d4b53 100644 --- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/TestEventsListener.java +++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/active/TestEventsListener.java @@ -129,7 +129,7 @@ try { subscriber.sync(); if (subscriber.getFailure() != null) { - throw HyracksDataException.create(subscriber.getFailure()); + throw subscriber.getFailure(); } } catch (Exception e) { throw HyracksDataException.create(e); diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/AbstractSubscriber.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/AbstractSubscriber.java index e21f9eb..880c4b4 100644 --- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/AbstractSubscriber.java +++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/AbstractSubscriber.java @@ -20,7 +20,6 @@ import org.apache.asterix.active.IActiveEntityEventSubscriber; import org.apache.asterix.active.IActiveEntityEventsListener; -import org.apache.hyracks.api.exceptions.HyracksDataException; public abstract class AbstractSubscriber implements IActiveEntityEventSubscriber { @@ -48,12 +47,9 @@ } @Override - public void sync() throws HyracksDataException, InterruptedException { + public void sync() throws InterruptedException { synchronized (listener) { while (!done) { - if (failure != null) { - throw HyracksDataException.create(failure); - } listener.wait(); } } -- To view, visit https://asterix-gerrit.ics.uci.edu/1986 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I85146028be70f4631d1ef2696489a4624bf23ad4 Gerrit-PatchSet: 2 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins Gerrit-Reviewer: Michael Blow Gerrit-Reviewer: Murtadha Hubail Gerrit-Reviewer: abdullah alamoudi