You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/02/22 11:05:52 UTC
[GitHub] [pulsar] sunxiaoguang opened a new pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
sunxiaoguang opened a new pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670
### Motivation
Refactor seek to reuse common logic so adding expression based seek later on would be easier.
### Modifications
*Describe the modifications you've done.*
Change seekAsync(long) and seekAsync(MessageId) in ConsumerImpl to share common code base.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
This change is already covered by existing tests
### Does this pull request potentially affect one of the following parts:
NA
### Documentation
NA
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#discussion_r580165968
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
##########
@@ -1761,31 +1761,31 @@ public void seek(long timestamp) throws PulsarClientException {
}
}
- @Override
- public CompletableFuture<Void> seekAsync(long timestamp) {
+ private Optional<CompletableFuture<Void>> seekAsyncCheckState(String seekBy) {
if (getState() == State.Closing || getState() == State.Closed) {
- return FutureUtil
- .failedFuture(new PulsarClientException.AlreadyClosedException(
- String.format("The consumer %s was already closed when seeking the subscription %s of the topic " +
- "%s to the timestamp %d", consumerName, subscription, topicName.toString(), timestamp)));
+ return Optional.of(FutureUtil
+ .failedFuture(new PulsarClientException.AlreadyClosedException(
+ String.format("The consumer %s was already closed when seeking the subscription %s of the"
+ + " topic %s to %s", consumerName, subscription, topicName.toString(), seekBy))));
}
if (!isConnected()) {
- return FutureUtil.failedFuture(new PulsarClientException(
- String.format("The client is not connected to the broker when seeking the subscription %s of the " +
- "topic %s to the timestamp %d", subscription, topicName.toString(), timestamp)));
+ return Optional.of(FutureUtil.failedFuture(new PulsarClientException(
+ String.format("The client is not connected to the broker when seeking the subscription %s of the "
+ + "topic %s to %s", subscription, topicName.toString(), seekBy))));
}
- final CompletableFuture<Void> seekFuture = new CompletableFuture<>();
+ return Optional.empty();
+ }
- long requestId = client.newRequestId();
- ByteBuf seek = Commands.newSeek(consumerId, requestId, timestamp);
+ private CompletableFuture<Void> seekAsyncFire(long requestId, ByteBuf seek, String seekBy) {
Review comment:
what about `seekAsyncInternal` ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sunxiaoguang commented on pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
sunxiaoguang commented on pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#issuecomment-784043655
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sunxiaoguang commented on a change in pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
sunxiaoguang commented on a change in pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#discussion_r580169354
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
##########
@@ -1761,31 +1761,31 @@ public void seek(long timestamp) throws PulsarClientException {
}
}
- @Override
- public CompletableFuture<Void> seekAsync(long timestamp) {
+ private Optional<CompletableFuture<Void>> seekAsyncCheckState(String seekBy) {
if (getState() == State.Closing || getState() == State.Closed) {
- return FutureUtil
- .failedFuture(new PulsarClientException.AlreadyClosedException(
- String.format("The consumer %s was already closed when seeking the subscription %s of the topic " +
- "%s to the timestamp %d", consumerName, subscription, topicName.toString(), timestamp)));
+ return Optional.of(FutureUtil
+ .failedFuture(new PulsarClientException.AlreadyClosedException(
+ String.format("The consumer %s was already closed when seeking the subscription %s of the"
+ + " topic %s to %s", consumerName, subscription, topicName.toString(), seekBy))));
}
if (!isConnected()) {
- return FutureUtil.failedFuture(new PulsarClientException(
- String.format("The client is not connected to the broker when seeking the subscription %s of the " +
- "topic %s to the timestamp %d", subscription, topicName.toString(), timestamp)));
+ return Optional.of(FutureUtil.failedFuture(new PulsarClientException(
+ String.format("The client is not connected to the broker when seeking the subscription %s of the "
+ + "topic %s to %s", subscription, topicName.toString(), seekBy))));
}
- final CompletableFuture<Void> seekFuture = new CompletableFuture<>();
+ return Optional.empty();
+ }
- long requestId = client.newRequestId();
- ByteBuf seek = Commands.newSeek(consumerId, requestId, timestamp);
+ private CompletableFuture<Void> seekAsyncFire(long requestId, ByteBuf seek, String seekBy) {
Review comment:
Sure, it's fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sunxiaoguang commented on pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
sunxiaoguang commented on pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#issuecomment-784046609
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sunxiaoguang commented on pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
sunxiaoguang commented on pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#issuecomment-783924753
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sunxiaoguang commented on pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
sunxiaoguang commented on pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#issuecomment-783867897
The CI-CPP, Python. Tests / cpp-tests task kept failing for quite some time. Maybe it's something related to docker image upgrade?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sunxiaoguang commented on pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
sunxiaoguang commented on pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#issuecomment-783846892
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sunxiaoguang removed a comment on pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
sunxiaoguang removed a comment on pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#issuecomment-783992983
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sunxiaoguang commented on pull request #9670: [pulsar-client] Refactor seek to reuse common logic.
Posted by GitBox <gi...@apache.org>.
sunxiaoguang commented on pull request #9670:
URL: https://github.com/apache/pulsar/pull/9670#issuecomment-783992983
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org