[jira] [Commented] (DRILL-5973) Support injection of time-bound pauses in server

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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

[ASF GitHub Bot (JIRA)]

[ 
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 Khatua 
Date:   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)