Make CLI requests wait for last command sequence number #2280

vcrfxia · 2018-12-15T00:44:23Z

Description

This PR introduces a new CLI flag requestPipelining that controls whether requests issued through the CLI will wait for previous requests issued through the CLI to complete before processing new requests. This behavior (requestPipelining=false) is the default, and is motivated by the goal to ensure read-after-write consistency for requests issued through the CLI. When requestPipelining=true, CLI requests will not wait for previous requests to complete and will instead begin execution immediately (the current behavior). The user can change between modes using a newly-added CLI-specific command: requestPipelining. Its usage is requestPipelining to show the current setting of the flag, and requestPipelining ON or requestPipelining OFF to set the flag to a particular value.

To implement the behavior of requestPipelining=false, the CLI now keeps track of the last sequence number of any request issued by the CLI, by updating this value as appropriate each time a response is received from the server. When requestPipelining=false, this sequence number is passed via commandSequenceNumber for the next request. Values of the requestPipelining flag and the last command sequence number of the CLI are reset if the server backing the CLI changes (via the server command).

Testing done

Added unit tests.

Reviewer checklist

Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
Ensure relevant issues are linked (description should include text like "Fixes #")

vcrfxia · 2018-12-15T00:50:26Z

This PR relates to #2268 which suggests we always default to using commandSequenceNumber=LAST_SEQUENCE_NUMBER, not just in the CLI but also when a REST request is made without an explicit commandSequenceNumber set. If we go with this option, then this PR would change to remove the addition of the special value LAST_SEQUENCE_NUMBER, since null would take on that meaning.

An open question if we go with the suggestion in #2268 is what the behavior should be if a CLI request times out while waiting for the command queue to catch up -- should we simply return the timeout error, or should we retry some number of times?

Regardless of whether we go with #2268 or not, my proposed implementation for how to wait for the command queue to catch up would be the one in this PR, which is why I have opened this PR now for feedback on the approach.

vcrfxia · 2018-12-15T00:54:36Z

ksql-rest-app/src/main/java/io/confluent/ksql/rest/server/computation/CommandStore.java

@@ -245,8 +251,18 @@ public void ensureConsumedPast(final long seqNum, final Duration timeout)
    return result;
  }

+  private long getSequenceNumberToWaitFor(final long seqNum) {
+    return seqNum == CommandStoreUtil.LAST_SEQUENCE_NUMBER ? lastCommandSequenceNumber : seqNum;


Instead of tracking lastCommandSequenceNumber and using that value here when seqNum == LAST_SEQUENCE_NUMBER, ideally we would call commandConsumer.endOffsets here directly, to truly get the current last sequence number in the command queue. However, commandConsumer does not support multi-threading and calls to this function are made by server threads issuing requests, in contrast to all other calls to commandConsumer which are made by the single command runner thread. Thus, tracking lastCommandSequenceNumber and using it here was the best workaround I could think of. Open to other suggestions!

Ask discussed in the meeting, this won't work as there is a race condition between the threads, i.e. its possible to receive a request that wasn't to be executed after all currently queued requests. So the code checks lastCommandSequenceNumber upon receiving the request, but it hasn't been updated since another command was posted, so its actually out of date... not good!

As discussed, use a separate thread with its own consumer. Use a blocking queue to post CompleteableFuture<Offset> to the thread. The thread can then block on the queue, then drain it, query the head offset and complete all the drained futures. Wrap all of this into a suitable class.

Yup, I'll implement the blocking queue approach in a separate PR. For now, I've retargeted this PR to focus on having CLI requests wait until all previously-issued requests (from the particular CLI) have finished, which @apurvam pointed out has a simpler implementation.

big-andy-coates

Thanks @vcrfxia

As discussed, I think the default behaviour if no command sequence is supplied should be to use the latest offset.

Also, there is a race condition in the current solution. but we've already discussed the solution.

Let me know when this is ready for another look.

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

ksql-cli/src/test/java/io/confluent/ksql/cli/CliTest.java

big-andy-coates · 2018-12-20T00:57:46Z

ksql-rest-app/src/main/java/io/confluent/ksql/rest/server/computation/CommandStore.java

@@ -245,8 +251,18 @@ public void ensureConsumedPast(final long seqNum, final Duration timeout)
    return result;
  }

+  private long getSequenceNumberToWaitFor(final long seqNum) {
+    return seqNum == CommandStoreUtil.LAST_SEQUENCE_NUMBER ? lastCommandSequenceNumber : seqNum;


Ask discussed in the meeting, this won't work as there is a race condition between the threads, i.e. its possible to receive a request that wasn't to be executed after all currently queued requests. So the code checks lastCommandSequenceNumber upon receiving the request, but it hasn't been updated since another command was posted, so its actually out of date... not good!

As discussed, use a separate thread with its own consumer. Use a blocking queue to post CompleteableFuture<Offset> to the thread. The thread can then block on the queue, then drain it, query the head offset and complete all the drained futures. Wrap all of this into a suitable class.

…r-write consistency differently.

vcrfxia

After discussions with @big-andy-coates, @hjafarpour and @apurvam , I've removed the parts of this PR for adding a special sequence number value LAST_SEQUENCE_NUMBER to indicate that the command queue should be fully caught up before executing the new request, and also changed the new CLI behavior in this PR from using the special value to instead keep track of the last sequence number of any request issued by the particular CLI instance, and issue subsequent commands using this value as the sequence number to wait for. I've updated the PR description accordingly.

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

vcrfxia · 2018-12-21T00:23:30Z

ksql-rest-app/src/main/java/io/confluent/ksql/rest/server/computation/CommandStore.java

@@ -245,8 +251,18 @@ public void ensureConsumedPast(final long seqNum, final Duration timeout)
    return result;
  }

+  private long getSequenceNumberToWaitFor(final long seqNum) {
+    return seqNum == CommandStoreUtil.LAST_SEQUENCE_NUMBER ? lastCommandSequenceNumber : seqNum;


Yup, I'll implement the blocking queue approach in a separate PR. For now, I've retargeted this PR to focus on having CLI requests wait until all previously-issued requests (from the particular CLI) have finished, which @apurvam pointed out has a simpler implementation.

vcrfxia · 2018-12-21T00:37:13Z

ksql-cli/src/test/java/io/confluent/ksql/cli/CliTest.java


 /**
 * Most tests in CliTest are end-to-end integration tests, so it may expect a long running time.
 */
+@RunWith(MockitoJUnitRunner.class)


It seems a little weird that this test has both integration tests and unit tests in it. (I switched the existing unit tests over to Mockito and also added more.) Would it make more sense to have separate files for the CLI integration and CLI units tests?

Will do in a follow-up. Thanks.

vcrfxia · 2018-12-21T00:40:28Z

ksql-cli/src/test/java/io/confluent/ksql/cli/CliTest.java

+  }
+
+  private CommandStatusEntity stubCommandStatusEntityWithSeqNum(final long seqNum) {
+    return new CommandStatusEntity(


I wanted this to return a mock and initially tried

private CommandStatusEntity mockCommandStatusEntityWithSeqNum(final long seqNum) { final CommandStatusEntity mockEntity = mock(CommandStatusEntity.class); when(mockEntity.getCommandSequenceNumber()).thenReturn(seqNum); when(mockEntity.getCommandStatus()).thenReturn(mock(CommandStatus.class)); return mockEntity; }

but the newly added tests were failing with

java.lang.RuntimeException: Unexpected KsqlEntity class: 'io.confluent.ksql.rest.entity.CommandStatusEntity$MockitoMock$564394681' at io.confluent.ksql.cli.console.Console.printAsTable(Console.java:350) at io.confluent.ksql.cli.console.Console.printKsqlEntityList(Console.java:294) at io.confluent.ksql.cli.Cli.printKsqlResponse(Cli.java:328) at io.confluent.ksql.cli.Cli.handleStatements(Cli.java:248) at io.confluent.ksql.cli.Cli.handleLine(Cli.java:181) at io.confluent.ksql.cli.CliTest.shouldUpdateCommandSequenceNumber(CliTest.java:865) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27) at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298) at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.lang.Thread.run(Thread.java:748)

which confuses me since Console.java certainly has a handler for the KsqlEntity CommandStatusEntity. Perhaps the class of the mock is subtly different from the class of a regular CommandStatusEntity?

What does .getClass on the mock return?

I think you'll find it's a different type, i.e. io.confluent.ksql.rest.entity.CommandStatusEntity$MockitoMock$564394681

So... rather than using a mock, can you just create a CommandStatusEntity in the right state?

Yup, that seems to be the problem, and your proposed solution is what stubCommandStatusEntityWithSeqNum implements. Good to know I wasn't just using Mockito incorrectly. Thanks!

big-andy-coates

Thanks @vcrfxia

Looking good 'cept I don't think we need a retry. See comment inline. This will also simplify the change even more ;)

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

big-andy-coates · 2019-01-09T18:54:41Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

+    final Optional<Long> lastSeqNum = entities.stream()
+        .filter(entity -> entity instanceof CommandStatusEntity)
+        .map(entity -> (CommandStatusEntity)entity)
+        .map(CommandStatusEntity::getCommandSequenceNumber)


Suggested change

.map(CommandStatusEntity::getCommandSequenceNumber)

.mapToLong(CommandStatusEntity::getCommandSequenceNumber)

.max()

.ifPresent(seqNum -> lastCommandSequenceNumber = seqNum)

I can't seem to get this proposed change to compile. mapToLong results in a stream of OptionalLongs, but max() only works on Optional<T>. Am I missing something?

Not sure what you're doing, but this works for me:

final long max = Stream.of("10") .mapToLong(Long::valueOf) .max().getAsLong();

:D

Yes, you're right. Sorry for the brain fart. (mapToLong results in a LongStream, to which max() can be applied just fine.) Fixed.

big-andy-coates · 2019-01-10T10:14:46Z

As discussed. Let's remove the retry and replace it with a more descriptive error message.

Secondly, let's add a CliSpecificCommand that allows the user to toggle between 'wait for previous command to finish' and 'fire and forget'. Include the details of this command in the error message the user sees if a request times out waiting for the previous command.

Finally, if the user switches servers within the CLI. (Via the server command), then we should reset the boolean that's tracking the fire-n-forget vs wait. Might be nice to have this flag in an object that the CLI tracks. When switching servers the object is just replaced with a new one, (which contains appropriate defaults). Then if we have any other such flags / state in the future it can be added here. Though... for the first and only flag this might be overkill!

vcrfxia

As discussed with @big-andy-coates and @rodesai offline (and summarized by @big-andy-coates above), the latest revision of this PR removes automatic retries from the CLI and instead implements a CLI-specific command wait-for-previous-command (suggestions for new names appreciated...) that can be used to control whether the CLI waits for previous commands to finish executing before making new requests. Its usage is wait-for-previous-command <ON/OFF>. The setting defaults to ON and resets when the server backing the CLI changes (via the server command).

The current setting of the flag is kept in the Cli. In the future we might like to move this flag (along with the Cli's lastCommandSequenceNumber) into an object tracked by the CLI as @big-andy-coates suggested above, since these are the two state values that need to be reset when the underlying server is changed.

The latest revision additionally changes the command queue catchup timeout used by StreamedQueryResource to be consistent with the timeouts used by KsqlResource and WSQueryEndpoint, since the previous discrepancy doesn't make sense.

vcrfxia · 2019-01-10T19:13:49Z

ksql-cli/src/test/java/io/confluent/ksql/cli/CliTest.java

+  }
+
+  private CommandStatusEntity stubCommandStatusEntityWithSeqNum(final long seqNum) {
+    return new CommandStatusEntity(


Yup, that seems to be the problem, and your proposed solution is what stubCommandStatusEntityWithSeqNum implements. Good to know I wasn't just using Mockito incorrectly. Thanks!

vcrfxia · 2019-01-10T19:14:13Z

ksql-cli/src/test/java/io/confluent/ksql/cli/CliTest.java


 /**
 * Most tests in CliTest are end-to-end integration tests, so it may expect a long running time.
 */
+@RunWith(MockitoJUnitRunner.class)


Will do in a follow-up. Thanks.

vcrfxia · 2019-01-10T19:15:11Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

+    final Optional<Long> lastSeqNum = entities.stream()
+        .filter(entity -> entity instanceof CommandStatusEntity)
+        .map(entity -> (CommandStatusEntity)entity)
+        .map(CommandStatusEntity::getCommandSequenceNumber)


I can't seem to get this proposed change to compile. mapToLong results in a stream of OptionalLongs, but max() only works on Optional<T>. Am I missing something?

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

big-andy-coates · 2019-01-11T20:09:43Z

Suggested name for the settings... "request pipelining". If turned on we don't wait for a response. This is the same as HTTP pipelining, so will be a familiar concept for some.

big-andy-coates

Looking good. Thanks @vcrfxia

Few comments below, but LGTM. There's a name suggestion in there too.

big-andy-coates · 2019-01-11T20:12:30Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

@@ -75,6 +78,9 @@
  private final KsqlRestClient restClient;
  private final Console terminal;

+  private long lastCommandSequenceNumber;
+  private boolean shouldWaitForPreviousCommand;


as per previous comment, consider

Suggested change

private boolean shouldWaitForPreviousCommand;

private boolean requestPipelining;

(Though note that this is the opposite of the current name, i.e. we should wait for previous command to finish if requestPipelining is false.)

big-andy-coates · 2019-01-11T20:17:49Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

@@ -75,6 +78,9 @@
  private final KsqlRestClient restClient;
  private final Console terminal;

+  private long lastCommandSequenceNumber;


I would suggest putting both lastCommandSequenceNumber and shouldWaitForPreviousCommand into a new object, (just an inner class), as both represent state associated with the remote server.

private static final RemoveServerState{ private long lastCommandSequenceNumber; private boolean requestPipelining; private RemoveServerState() { reset(); } private void reset() { lastCommandSequenceNumber = -1L requestPipelining = false; } }

If/when the user switches to a different KSQL server, (using the server command), both of these should be reset. Having in a separate ServerState object, with a reset method that is called when switching servers and in its own the constructor, should ensure a nice pattern for ensuring bad state does not persist when switching servers.

Done, except doing so violated the class data abstraction coupling limit for Cli.java, which I fixed by rearranging the various calls to registerCliSpecificCommand (specifically, moving them all into CliCommandRegisterUtil. Unfortunately this resulted in the class data abstraction coupling limit for CliCommandRegisterUtil being exceeded, but I think that's OK given the nature of the class?

big-andy-coates · 2019-01-11T20:19:18Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

+    final Optional<Long> lastSeqNum = entities.stream()
+        .filter(entity -> entity instanceof CommandStatusEntity)
+        .map(entity -> (CommandStatusEntity)entity)
+        .map(CommandStatusEntity::getCommandSequenceNumber)


Not sure what you're doing, but this works for me:

final long max = Stream.of("10") .mapToLong(Long::valueOf) .max().getAsLong();

:D

big-andy-coates · 2019-01-11T20:20:20Z

ksql-cli/src/main/java/io/confluent/ksql/cli/console/cmd/RemoteServerSpecificCommand.java


  public RemoteServerSpecificCommand(
      final KsqlRestClient restClient,
-      final PrintWriter writer) {
+      final PrintWriter writer,
+      final Consumer<Void> resetCliForNewServer) {


We should just add a generic 'Event' interface to our common lib. Do you fancy doing it? :D

Done. Slipped it in as ksql-common/src/main/java/io/confluent/ksql/util/Event.java. (Would another subfolder be more appropriate?)

big-andy-coates · 2019-01-11T20:20:47Z

ksql-cli/src/main/java/io/confluent/ksql/cli/console/cmd/WaitForPreviousCommand.java

+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
+public class WaitForPreviousCommand implements CliSpecificCommand {


rename to RequestPipeliningCommand obvs

big-andy-coates · 2019-01-11T20:24:02Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

+          + "If you wish to execute new commands without waiting for "
+          + "prior commands to finish, run the command '%s OFF'.%n",
+          WaitForPreviousCommand.NAME);
+    } else if (isKsqlEntityList(response)) {


what about if response is a plain KsqlEntity? (Is this possible?)

Not currently possible, since KsqlRestClient.makeKsqlRequest() returns RestResponse<KsqlEntityList>. Not sure if there's a good way to set up my change to prevent errors, should this no longer be the case in the future... open to suggestions.

vcrfxia

Thanks for the review @big-andy-coates ! Implemented your suggestions.

A little weirdness that may be worth looking at again is how I've refactored where various CLI-specific commands get registered. Previously, help, clear, output, history, version, and exit were being registered in the Console, while server and requestPipelining were registered in the Cli. I've moved version to be registered in the Cli since it feels it belongs on that side of the Console/Cli divide. But perhaps we shouldn't have the divide at all.

vcrfxia · 2019-01-11T22:41:34Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

@@ -75,6 +78,9 @@
  private final KsqlRestClient restClient;
  private final Console terminal;

+  private long lastCommandSequenceNumber;


Done, except doing so violated the class data abstraction coupling limit for Cli.java, which I fixed by rearranging the various calls to registerCliSpecificCommand (specifically, moving them all into CliCommandRegisterUtil. Unfortunately this resulted in the class data abstraction coupling limit for CliCommandRegisterUtil being exceeded, but I think that's OK given the nature of the class?

vcrfxia · 2019-01-11T22:41:44Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

@@ -75,6 +78,9 @@
  private final KsqlRestClient restClient;
  private final Console terminal;

+  private long lastCommandSequenceNumber;
+  private boolean shouldWaitForPreviousCommand;


vcrfxia · 2019-01-11T22:43:05Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

+          + "If you wish to execute new commands without waiting for "
+          + "prior commands to finish, run the command '%s OFF'.%n",
+          WaitForPreviousCommand.NAME);
+    } else if (isKsqlEntityList(response)) {


Not currently possible, since KsqlRestClient.makeKsqlRequest() returns RestResponse<KsqlEntityList>. Not sure if there's a good way to set up my change to prevent errors, should this no longer be the case in the future... open to suggestions.

vcrfxia · 2019-01-11T22:44:18Z

ksql-cli/src/main/java/io/confluent/ksql/cli/Cli.java

+    final Optional<Long> lastSeqNum = entities.stream()
+        .filter(entity -> entity instanceof CommandStatusEntity)
+        .map(entity -> (CommandStatusEntity)entity)
+        .map(CommandStatusEntity::getCommandSequenceNumber)


Yes, you're right. Sorry for the brain fart. (mapToLong results in a LongStream, to which max() can be applied just fine.) Fixed.

vcrfxia · 2019-01-11T22:45:42Z

ksql-cli/src/main/java/io/confluent/ksql/cli/console/cmd/RemoteServerSpecificCommand.java


  public RemoteServerSpecificCommand(
      final KsqlRestClient restClient,
-      final PrintWriter writer) {
+      final PrintWriter writer,
+      final Consumer<Void> resetCliForNewServer) {


Done. Slipped it in as ksql-common/src/main/java/io/confluent/ksql/util/Event.java. (Would another subfolder be more appropriate?)

vcrfxia · 2019-01-11T22:45:53Z

ksql-cli/src/main/java/io/confluent/ksql/cli/console/cmd/WaitForPreviousCommand.java

+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
+public class WaitForPreviousCommand implements CliSpecificCommand {


blueedgenick · 2019-01-11T23:58:10Z

A little weirdness that may be worth looking at again is how I've refactored where various CLI-specific commands get registered. Previously, help, clear, output, history, version, and exit were being registered in the Console, while server and requestPipelining were registered in the Cli. I've moved version to be registered in the Cli since it feels it belongs on that side of the Console/Cli divide. But perhaps we shouldn't have the divide at all.

Non-binding comment: i've personally always found the whole cli/console/terminal thing to be confusing and unnecessary when i've been in there too, perhaps there's a consensus on how it could be better ?

rodesai

LGTM. Great tests!

vcrfxia · 2019-01-15T17:36:46Z

Thanks for the feedback and reviews, all! I like this final version so much better than the first.

Updated the PR description to describe the latest version. Will hold off to merge until #2331 gets in since merge conflicts should be easier to resolve on this side.

vcrfxia requested a review from a team as a code owner December 15, 2018 00:44

vcrfxia commented Dec 15, 2018

View reviewed changes

vcrfxia changed the title ~~make CLI requests wait for last command sequence number.~~ Make CLI requests wait for last command sequence number Dec 15, 2018

big-andy-coates reviewed Dec 20, 2018

View reviewed changes

big-andy-coates requested a review from a team December 20, 2018 00:59

vcrfxia added 2 commits December 20, 2018 14:43

make CLI requests wait for last command sequence number.

f0cb4c0

back out changes for waiting for last offset. implement CLI read-afte…

8a96569

…r-write consistency differently.

vcrfxia commented Dec 21, 2018

View reviewed changes

vcrfxia force-pushed the cli-request-indices branch from 9030ad7 to 8a96569 Compare December 21, 2018 00:32

vcrfxia commented Dec 21, 2018

View reviewed changes

vcrfxia requested a review from big-andy-coates December 21, 2018 00:43

Merge branch 'master' into cli-request-indices

c15eb27

big-andy-coates reviewed Jan 9, 2019

View reviewed changes

vcrfxia added 6 commits January 10, 2019 11:26

implement new CLI-specific command, rather than retrying automatically.

db443d9

separate command queue catch timeout from disconnect check interval.

d2b7e0c

Merge branch 'master' into cli-request-indices

38e8079

reset lastCommandSequenceNumber upon server change.

dc0119a

refactor and add test.

3cd3dee

rename new cli command.

cb151cc

vcrfxia commented Jan 10, 2019

View reviewed changes

remove unneeded diff.

da72ba6

vcrfxia requested review from big-andy-coates and a team January 10, 2019 21:52

big-andy-coates approved these changes Jan 11, 2019

View reviewed changes

andy's requested changes.

93b49e3

vcrfxia commented Jan 11, 2019

View reviewed changes

vcrfxia added 2 commits January 11, 2019 15:23

missed a spot in switching CLI command to new name.

67b1b3e

better docs.

75695a0

Merge branch 'master' into cli-request-indices

86a1452

rodesai approved these changes Jan 15, 2019

View reviewed changes

Merge branch 'master' into cli-request-indices

614c4df

vcrfxia added 5 commits January 16, 2019 12:28

Merge branch 'master' into cli-request-indices

6fb5f53

Merge branch 'master' into cli-request-indices

6613596

fix test

637c6bc

Merge branch 'master' into cli-request-indices

30e3bba

update year.

1e4c2b2

vcrfxia merged commit ae10877 into confluentinc:master Feb 1, 2019

vcrfxia added this to the 5.2 milestone Feb 19, 2019

vcrfxia mentioned this pull request Jun 11, 2019

RUN SCRIPT - Provide way to wait for a stream/topic to exist before continuing #2880

Closed

vcrfxia deleted the cli-request-indices branch May 18, 2020 16:32

-        .map(CommandStatusEntity::getCommandSequenceNumber)
+        .mapToLong(CommandStatusEntity::getCommandSequenceNumber)
+        .max()
+        .ifPresent(seqNum -> lastCommandSequenceNumber = seqNum)

	private boolean shouldWaitForPreviousCommand;
	private boolean requestPipelining;

Make CLI requests wait for last command sequence number #2280

Make CLI requests wait for last command sequence number #2280

Conversation

vcrfxia commented Dec 15, 2018 • edited Loading

Description

Testing done

Reviewer checklist

vcrfxia commented Dec 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

big-andy-coates left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vcrfxia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

big-andy-coates left a comment

Choose a reason for hiding this comment

big-andy-coates Jan 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

big-andy-coates commented Jan 10, 2019

vcrfxia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

big-andy-coates commented Jan 11, 2019

big-andy-coates left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vcrfxia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueedgenick commented Jan 11, 2019

rodesai left a comment

Choose a reason for hiding this comment

vcrfxia commented Jan 15, 2019 • edited Loading

vcrfxia commented Dec 15, 2018 •

edited

Loading

big-andy-coates Jan 9, 2019 •

edited

Loading

vcrfxia commented Jan 15, 2019 •

edited

Loading