[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16308157#comment-16308157 ] ASF GitHub Bot commented on DRILL-5973: --- Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/1055 > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Labels: ready-to-commit > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16299072#comment-16299072 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/1055 Didn't make sense to have small commits in the PR. So I've squashed and rebased onto 1.13.0. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Labels: ready-to-commit > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16289776#comment-16289776 ] ASF GitHub Bot commented on DRILL-5973: --- Github user parthchandra commented on the issue: https://github.com/apache/drill/pull/1055 +1. No need to squash/rebase. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16287204#comment-16287204 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/1055 @parthchandra Passed all regression and unit tests. Travis build is failing due to the GraceFullShutdown tests. Let me know if I should squash and rebase. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16284634#comment-16284634 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/1055 @parthchandra , the code was already rebased. The failing unit tests were related to TestCountDownLatchInjection, which had APIs that were unnecessarily extended to incorporate pause durations. Reverting it back to the original constructor resolved the issues and both, previously failing unit tests passed. Waiting for the rest of the regressions to pass. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16284223#comment-16284223 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r155877298 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjectionImpl.java --- @@ -41,8 +41,11 @@ private CountDownLatchInjectionImpl(@JsonProperty("address") final String address, @JsonProperty("port") final int port, @JsonProperty("siteClass") final String siteClass, - @JsonProperty("desc") final String desc) throws InjectionConfigurationException { -super(address, port, siteClass, desc, 0, 1); + @JsonProperty("desc") final String desc, + @JsonProperty("nSkip") final int nSkip, + @JsonProperty("nFire") final int nFire, + @JsonProperty("msPause") final Long msPause) throws InjectionConfigurationException { --- End diff -- Might have hit it accidentally. Don't think I need a null check. Will walk through this once and update it. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16284225#comment-16284225 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/1055 Shouldn't be failing. I think I'll rebase and run pre-commit tests to confirm. Thanks! > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16281165#comment-16281165 ] ASF GitHub Bot commented on DRILL-5973: --- Github user parthchandra commented on the issue: https://github.com/apache/drill/pull/1055 This PR seems to be failing a bunch of unit that don't seem related. do you need to rebase ? > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16280658#comment-16280658 ] ASF GitHub Bot commented on DRILL-5973: --- Github user parthchandra commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r155326676 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjectionImpl.java --- @@ -41,8 +41,11 @@ private CountDownLatchInjectionImpl(@JsonProperty("address") final String address, @JsonProperty("port") final int port, @JsonProperty("siteClass") final String siteClass, - @JsonProperty("desc") final String desc) throws InjectionConfigurationException { -super(address, port, siteClass, desc, 0, 1); + @JsonProperty("desc") final String desc, + @JsonProperty("nSkip") final int nSkip, + @JsonProperty("nFire") final int nFire, + @JsonProperty("msPause") final Long msPause) throws InjectionConfigurationException { --- End diff -- long instead of Long? You will save a lot of unnecessary checking for null > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273822#comment-16273822 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/1055 Verified the PauseInjection tests and that the commit builds and works normally. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273802#comment-16273802 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/1055 Sorry. Pushing in changes and squashing simultaneously. Made the relevant changes, except for the check ``` long pauseDuration = pauseInjection.getMsPause(); if ( pauseDuration > 0L) { ``` > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273796#comment-16273796 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154248904 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java --- @@ -144,6 +144,34 @@ public void pauseInjected() { } } + @Test + public void timedPauseInjected() { +final long pauseDuration = 2000L; +final long expectedDuration = pauseDuration; +final ExtendedLatch trigger = new ExtendedLatch(1); +final Pointer ex = new Pointer<>(); +final String controls = Controls.newBuilder() + .addTimedPause(DummyClass.class, DummyClass.PAUSES, 0, pauseDuration) + .build(); + +ControlsInjectionUtil.setControls(session, controls); + +final QueryContext queryContext = new QueryContext(session, bits[0].getContext(), QueryId.getDefaultInstance()); +//We don't need a ResumingThread because this should automatically resume + +// test that the pause happens +final DummyClass dummyClass = new DummyClass(queryContext, trigger); +final long actualDuration = dummyClass.pauses(); +assertTrue(String.format("Test should stop for at least %d milliseconds.", expectedDuration), + expectedDuration <= actualDuration); +assertTrue("No exception should be thrown.", ex.value == null); +try { + queryContext.close(); +} catch (final Exception e) { + fail("Failed to close query context: " + e); --- End diff -- ``` did you forget to remove that part following up on your other change? ``` Didn't forget to remove. It drives the logger to publish the message in the debugger as time-bound if it is non-zero. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273790#comment-16273790 ] ASF GitHub Bot commented on DRILL-5973: --- Github user laurentgo commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154247977 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java --- @@ -144,6 +144,34 @@ public void pauseInjected() { } } + @Test + public void timedPauseInjected() { +final long pauseDuration = 2000L; +final long expectedDuration = pauseDuration; +final ExtendedLatch trigger = new ExtendedLatch(1); +final Pointer ex = new Pointer<>(); +final String controls = Controls.newBuilder() + .addTimedPause(DummyClass.class, DummyClass.PAUSES, 0, pauseDuration) + .build(); + +ControlsInjectionUtil.setControls(session, controls); + +final QueryContext queryContext = new QueryContext(session, bits[0].getContext(), QueryId.getDefaultInstance()); +//We don't need a ResumingThread because this should automatically resume + +// test that the pause happens +final DummyClass dummyClass = new DummyClass(queryContext, trigger); +final long actualDuration = dummyClass.pauses(); +assertTrue(String.format("Test should stop for at least %d milliseconds.", expectedDuration), + expectedDuration <= actualDuration); +assertTrue("No exception should be thrown.", ex.value == null); +try { + queryContext.close(); +} catch (final Exception e) { + fail("Failed to close query context: " + e); --- End diff -- maybe better let JUnit handle unexpected expection (at least you would get the whole stack trace instead of just the error message) (I guess it's not new code, so feel free to ignore) > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273789#comment-16273789 ] ASF GitHub Bot commented on DRILL-5973: --- Github user laurentgo commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154248201 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java --- @@ -81,8 +83,20 @@ public void injectPause(final ExecutionControls executionControls, final String executionControls.lookupPauseInjection(this, desc); if (pauseInjection != null) { - logger.debug("Pausing at {}", desc); - pauseInjection.pause(); + long pauseDuration = pauseInjection.getMsPause(); + if ( pauseDuration > 0L) { +logger.debug("Pausing at {} for {} sec", desc, TimeUnit.MILLISECONDS.toSeconds(pauseDuration) ); + } else { +logger.debug("Pausing at {}", desc); + } + try { +pauseInjection.pause(); + } catch (InterruptedException e) { +//Unpause if this is a timed pause, because an explicit unpause is not called +if (pauseDuration > 0L) { --- End diff -- did you forget to remove that part following up on your other change? > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273783#comment-16273783 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154247369 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java --- @@ -44,14 +46,31 @@ private PauseInjection(@JsonProperty("address") final String address, @JsonProperty("siteClass") final String siteClass, @JsonProperty("desc") final String desc, @JsonProperty("nSkip") final int nSkip) throws InjectionConfigurationException { -super(address, port, siteClass, desc, nSkip, 1); +//nFire is 1 since we will inject pauses only once +//msPause is 0 (i.e. not time-bound) +super(address, port, siteClass, desc, nSkip, 1, 0L); + } + + @JsonCreator // ensures instances are created only through JSON + private PauseInjection(@JsonProperty("address") final String address, + @JsonProperty("port") final int port, + @JsonProperty("siteClass") final String siteClass, + @JsonProperty("desc") final String desc, + @JsonProperty("nSkip") final int nSkip, + @JsonProperty("msPause") final long msPause) throws InjectionConfigurationException { +//nFire is 1 since we will inject pauses only once +super(address, port, siteClass, desc, nSkip, 1, msPause); } - public void pause() { + public void pause() throws InterruptedException { --- End diff -- Agreed. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273785#comment-16273785 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154247418 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java --- @@ -81,8 +83,20 @@ public void injectPause(final ExecutionControls executionControls, final String executionControls.lookupPauseInjection(this, desc); if (pauseInjection != null) { - logger.debug("Pausing at {}", desc); - pauseInjection.pause(); + long pauseDuration = pauseInjection.getMsPause(); + if ( pauseDuration > 0L) { +logger.debug("Pausing at {} for {} sec", desc, TimeUnit.MILLISECONDS.toSeconds(pauseDuration) ); + } else { +logger.debug("Pausing at {}", desc); + } + try { +pauseInjection.pause(); + } catch (InterruptedException e) { +//Unpause if this is a timed pause, because an explicit unpause is not called +if (pauseDuration > 0L) { --- End diff -- Will make the change since the InterruptedException is being handled in the timed pause (based on your comment below). > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273777#comment-16273777 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154246723 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java --- @@ -81,8 +83,20 @@ public void injectPause(final ExecutionControls executionControls, final String executionControls.lookupPauseInjection(this, desc); if (pauseInjection != null) { - logger.debug("Pausing at {}", desc); - pauseInjection.pause(); + long pauseDuration = pauseInjection.getMsPause(); + if ( pauseDuration > 0L) { +logger.debug("Pausing at {} for {} sec", desc, TimeUnit.MILLISECONDS.toSeconds(pauseDuration) ); + } else { +logger.debug("Pausing at {}", desc); + } + try { +pauseInjection.pause(); + } catch (InterruptedException e) { +//Unpause if this is a timed pause, because an explicit unpause is not called +if (pauseDuration > 0L) { --- End diff -- Just following convention of 'unpausing' explicitly, rather than implicitly within the `pause()` for a limited-duration pause. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273772#comment-16273772 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154246413 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java --- @@ -81,8 +83,20 @@ public void injectPause(final ExecutionControls executionControls, final String executionControls.lookupPauseInjection(this, desc); if (pauseInjection != null) { - logger.debug("Pausing at {}", desc); - pauseInjection.pause(); + long pauseDuration = pauseInjection.getMsPause(); + if ( pauseDuration > 0L) { --- End diff -- Agreed, but the only purpose of that check is to indicate during debugging that a non-zero timed pause was injected. Hence, the check. > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273075#comment-16273075 ] ASF GitHub Bot commented on DRILL-5973: --- Github user laurentgo commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154153939 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java --- @@ -44,14 +46,31 @@ private PauseInjection(@JsonProperty("address") final String address, @JsonProperty("siteClass") final String siteClass, @JsonProperty("desc") final String desc, @JsonProperty("nSkip") final int nSkip) throws InjectionConfigurationException { -super(address, port, siteClass, desc, nSkip, 1); +//nFire is 1 since we will inject pauses only once +//msPause is 0 (i.e. not time-bound) +super(address, port, siteClass, desc, nSkip, 1, 0L); + } + + @JsonCreator // ensures instances are created only through JSON + private PauseInjection(@JsonProperty("address") final String address, + @JsonProperty("port") final int port, + @JsonProperty("siteClass") final String siteClass, + @JsonProperty("desc") final String desc, + @JsonProperty("nSkip") final int nSkip, + @JsonProperty("msPause") final long msPause) throws InjectionConfigurationException { +//nFire is 1 since we will inject pauses only once +super(address, port, siteClass, desc, nSkip, 1, msPause); } - public void pause() { + public void pause() throws InterruptedException { --- End diff -- I would not expose the interruption but handle it internally instead (by decreasing the latch counter) > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273077#comment-16273077 ] ASF GitHub Bot commented on DRILL-5973: --- Github user laurentgo commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154150476 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java --- @@ -81,8 +83,20 @@ public void injectPause(final ExecutionControls executionControls, final String executionControls.lookupPauseInjection(this, desc); if (pauseInjection != null) { - logger.debug("Pausing at {}", desc); - pauseInjection.pause(); + long pauseDuration = pauseInjection.getMsPause(); + if ( pauseDuration > 0L) { --- End diff -- maybe nitpicky but I would use -1 (or any negative value) to distinguish between timed-bounded pause or not (since 0 is a valid wait time for CountDownLatch) > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16273076#comment-16273076 ] ASF GitHub Bot commented on DRILL-5973: --- Github user laurentgo commented on a diff in the pull request: https://github.com/apache/drill/pull/1055#discussion_r154153787 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java --- @@ -81,8 +83,20 @@ public void injectPause(final ExecutionControls executionControls, final String executionControls.lookupPauseInjection(this, desc); if (pauseInjection != null) { - logger.debug("Pausing at {}", desc); - pauseInjection.pause(); + long pauseDuration = pauseInjection.getMsPause(); + if ( pauseDuration > 0L) { +logger.debug("Pausing at {} for {} sec", desc, TimeUnit.MILLISECONDS.toSeconds(pauseDuration) ); + } else { +logger.debug("Pausing at {}", desc); + } + try { +pauseInjection.pause(); + } catch (InterruptedException e) { +//Unpause if this is a timed pause, because an explicit unpause is not called +if (pauseDuration > 0L) { --- End diff -- maybe to be done in PauseInjection (seems like something internal) > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16271433#comment-16271433 ] ASF GitHub Bot commented on DRILL-5973: --- Github user kkhatua commented on the issue: https://github.com/apache/drill/pull/1055 @laurentgo / @parthchandra Please review this. It is the basis for unit tests in DRILL-3640 > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server
[ https://issues.apache.org/jira/browse/DRILL-5973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16271431#comment-16271431 ] ASF GitHub Bot commented on DRILL-5973: --- GitHub user kkhatua opened a pull request: https://github.com/apache/drill/pull/1055 DRILL-5973 : Support injection of time-bound pauses in server Support pause injections in the test framework that are time-bound, to allow for testing high latency scenarios. e.g. delayed server response to the Drill client allows for test a server-induced timeout This would allow for testing of DRILL-3640 You can merge this pull request into a Git repository by running: $ git pull https://github.com/kkhatua/drill DRILL-5973 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1055.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1055 commit 62e6f721183d648797d5329e94b277cd5722bba6 Author: Kunal KhatuaDate: 2017-11-29T19:00:11Z DRILL-5973 : Support injection of time-bound pauses in server Support pause injections in the test framework that are time-bound, to allow for testing high latency scenarios. e.g. delayed server response to the Drill client allows for test a server-induced timeout > Support injection of time-bound pauses in server > > > Key: DRILL-5973 > URL: https://issues.apache.org/jira/browse/DRILL-5973 > Project: Apache Drill > Issue Type: Improvement > Components: Tools, Build & Test >Affects Versions: 1.11.0 >Reporter: Kunal Khatua >Assignee: Kunal Khatua > Fix For: 1.13.0 > > > While working on DRILL-3640 , when creating a unit test for a server-induced > timeout, the injecting a pause leaves the JUnit framework's DrillClient > without a handle to the query on the server. This is because we injected the > pause to occur before the server could send back a query ID, so the > DrillClient has no way to unpause the server. > The workaround to support this unit test is to allow for injecting pauses > with a defined time-bound, after which the server would resume. -- This message was sent by Atlassian JIRA (v6.4.14#64029)