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/09/08 03:51:07 UTC
[GitHub] [pulsar] Jason918 opened a new pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Jason918 opened a new pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960
Implementation of [PIP-63](https://github.com/apache/pulsar/wiki/PIP-63%3A-Readonly-Topic-Ownership-Support)
### Motivation
See [PIP-63](https://github.com/apache/pulsar/wiki/PIP-63%3A-Readonly-Topic-Ownership-Support)
### Modifications
As described in [PIP-63](https://github.com/apache/pulsar/wiki/PIP-63%3A-Readonly-Topic-Ownership-Support).
This pr comes with these key changes.
1. First of all, readeonly topic, more pricisely readonly namespace, is defined by `writerNamespace` in `org.apache.pulsar.common.policies.data.Policies` of this namespace.
For example, when `writerNamespace` is set as 'NS-W' in the `Policies` of a namespace 'NS-R', we can consume all the topics from namespace 'NS-W' through namespace 'NS-R'. If a consumer client subscribes a reader topic "persistent://tenant/NS-R/topic", it's effectively subscribing the writer topic "persistent://tenant/NS-W/topic".
Using namespace level readonly configuration instead of topic level configuration, we can use namespace isolation strategy to separate one brokers clusters to writer-brokers and readonly-brokers.
2. How readonly topic owner read data
Related logic is mostly handled in `org.apache.bookkeeper.mledger.impl.RemoteManagedLedgerImpl` which extends `ManagedLedgerImpl` for readonly topics, and changed some key behaviors, including
- Forbid all entry modification ops, like 'addEntry' and 'truncate'.
- Read all ledger info from zk, and update LAC by call `org.apache.bookkeeper.client.api.ReadHandle#readLastAddConfirmedAsync` of the last ledger in the ledger list. See method `RemoteManagedLedgerImpl#asyncUpdateLastConfirmedEntry()` for details.
- Using a `org.apache.pulsar.client.api.CursorClient` for managing cursors in writer topic. And it's RemoteManagedCursorImpl which extends ManagedCursorImpl used for cursor management in readonly topic.
- For consumers, each time `RemoteManagedCursorImpl#checkForNewEntries` will trigger `asyncUpdateLastConfirmedEntry` to update LAC from bookies. It's not quite efficient for now, but I look forward to optimize this to a long polling way from broker (or bookie) in the following PRs, since this PR is already quite big for reviewers.
3. How does readonly topic owner keep metadata in-sync
Add a method `watchManagedLedgerInfo` in `org.apache.bookkeeper.mledger.impl.MetaStore`.
Every `RemoteManagedLedgerImpl` will watch its ManagedLedgerInfo to see if there is a new ledger rolled.
4. How does readonly topic owner handle acknowledgment
A few cursor related RPCs is added for the acknowledgment including
- GET_CURSOR
- CREATE_CURSOR
- DELETE_CURSOR
- UPDATE_CURSOR
See `pulsar-common/src/main/proto/PulsarApi.proto` for details.
A CursorClient is added for readonly topics to use these apis.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
- RemoteManagedCursorImplTest.java
- RemoteManagedLedgerImplTest.java
- CursorClientTest.java
- ReadonlyTopicOwnerTest.java
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (no)
### Documentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
- [ ] doc-required
(If you need help on updating docs, create a doc issue)
- [ ] no-need-doc
(Please explain why)
- [ ] doc
(If this PR contains doc changes)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 closed pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 closed pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-916788459
@Jason918 thanks for your info!
I see you've already added some how-to instructions, so I label this PR with `doc`.
Please do not forget to add docs later to allow users to know your great code changes. And you can ping me to review the docs, thanks.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#discussion_r704305787
##########
File path: pulsar-client-api/src/main/java/org/apache/pulsar/client/api/CursorClient.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.api;
+
+import java.io.Closeable;
+import java.util.concurrent.CompletableFuture;
+import org.apache.pulsar.common.classification.InterfaceAudience;
+import org.apache.pulsar.common.classification.InterfaceStability;
+
+/**
+ * A {@link CursorClient} connects to a PersistentTopic and can manage cursor data of a persistent subscription.
+ * This is an interface that abstracts behavior of Pulsar's cursor client.
+ * This {@link CursorClient} is used by a readonly topic owner.
+ *
+ * @since 2.9.0
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Evolving
+public interface CursorClient extends Closeable {
Review comment:
this is a new API and it is not cited in PIP-63
https://github.com/apache/pulsar/wiki/PIP-63%3A-Readonly-Topic-Ownership-Support
can you please update the document or start a new one ?
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/Policies.java
##########
@@ -125,6 +125,12 @@
@SuppressWarnings("checkstyle:MemberName")
public String resource_group_name = null;
+ /**
+ * if writer_namespace is set, it means this is a readonly namespace. All data is read from writer_namespace
+ */
+ public String writerNamespace = null;
Review comment:
I believe that this is a little bit confusing an error prone.
can we add a "readonlyModeEnabled" flag ? (or with a better name)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-914990934
> You can apply following diff to fix C++ client build failure.
>
> ```diff
> diff --git a/pulsar-client-cpp/lib/Commands.cc b/pulsar-client-cpp/lib/Commands.cc
> index 87c149c5b87..760549aaa31 100644
> --- a/pulsar-client-cpp/lib/Commands.cc
> +++ b/pulsar-client-cpp/lib/Commands.cc
> @@ -642,6 +642,18 @@ std::string Commands::messageType(BaseCommand_Type type) {
> case BaseCommand::END_TXN_ON_SUBSCRIPTION_RESPONSE:
> return "END_TXN_ON_SUBSCRIPTION_RESPONSE";
> break;
> + case BaseCommand::GET_CURSOR:
> + return "GET_CURSOR";
> + case BaseCommand::GET_CURSOR_RESPONSE:
> + return "GET_CURSOR_RESPONSE";
> + case BaseCommand::CREATE_CURSOR:
> + return "CREATE_CURSOR";
> + case BaseCommand::CREATE_CURSOR_RESPONSE:
> + return "CREATE_CURSOR_RESPONSE";
> + case BaseCommand::DELETE_CURSOR:
> + return "DELETE_CURSOR";
> + case BaseCommand::UPDATE_CURSOR:
> + return "UPDATE_CURSOR";
> };
> BOOST_THROW_EXCEPTION(std::logic_error("Invalid BaseCommand enumeration value"));
> }
> ```
Thanks for your patch. :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-915650955
@Jason918 Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a change in pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#discussion_r713832421
##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
##########
@@ -1850,6 +1831,10 @@ public void operationFailed(ManagedLedgerException exception) {
});
}
+ protected void persistPosition(MarkDeleteEntry mdEntry, VoidCallback callback) {
Review comment:
`RemoteManagedCursorImpl` overrided this method and use CursorClient to persist this 'mdEntry' to topic write owner.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a change in pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#discussion_r706097157
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/Policies.java
##########
@@ -125,6 +125,12 @@
@SuppressWarnings("checkstyle:MemberName")
public String resource_group_name = null;
+ /**
+ * if writer_namespace is set, it means this is a readonly namespace. All data is read from writer_namespace
+ */
+ public String writerNamespace = null;
Review comment:
@eolivelli Thank you for pointing this out, totally agree with you.
Naming is always the difficult part. 'Readonly' is a little confusing in this project as there is already a 'ReadonlyManagedLedger', but it's completely different aspect of readonly.
Added "shadowReaderModeEnabled" for this flag. Is that ok?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-915066569
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-914990394
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-1031121110
This PR is too large, and very hard to review throughly.
Plan to do some rework and split this apart with PRs.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-916579087
> @Jason918 Thanks for your contribution. For this PR, do we need to update docs?
>
> (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
The PR template says that `if you have committer privilege`, maybe I didn't get that right. I thought it's for committers ( with write access to the project ) to decide about the docs.
About user doc, This PR is not ready for end users to use this feature in a very convenient way. For example, some parameters is added in namespace policies, but it is not covered in pulsar-admin or REST admin interface.
I would like to add these admin tools and user docs in the following PRs.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-917770759
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-916579508
> @Jason918 Thanks for your contribution. For this PR, do we need to update docs?
>
> (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
Sorry for that, the PR template says that `if you have committer privilege`, maybe I didn't get that right. I thought it's for committers ( with write access to the project ) to decide about the docs.
About user doc, This PR is not ready for end users to use this feature in a very convenient way. For example, some parameters is added in namespace policies, but it is not covered in pulsar-admin or REST admin interface.
I would like to add these admin tools and user docs in the following PRs.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 removed a comment on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 removed a comment on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-916579087
> @Jason918 Thanks for your contribution. For this PR, do we need to update docs?
>
> (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
The PR template says that `if you have committer privilege`, maybe I didn't get that right. I thought it's for committers ( with write access to the project ) to decide about the docs.
About user doc, This PR is not ready for end users to use this feature in a very convenient way. For example, some parameters is added in namespace policies, but it is not covered in pulsar-admin or REST admin interface.
I would like to add these admin tools and user docs in the following PRs.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-914908483
You can apply following diff to fix C++ client build failure.
```diff
diff --git a/pulsar-client-cpp/lib/Commands.cc b/pulsar-client-cpp/lib/Commands.cc
index 87c149c5b87..760549aaa31 100644
--- a/pulsar-client-cpp/lib/Commands.cc
+++ b/pulsar-client-cpp/lib/Commands.cc
@@ -642,6 +642,18 @@ std::string Commands::messageType(BaseCommand_Type type) {
case BaseCommand::END_TXN_ON_SUBSCRIPTION_RESPONSE:
return "END_TXN_ON_SUBSCRIPTION_RESPONSE";
break;
+ case BaseCommand::GET_CURSOR:
+ return "GET_CURSOR";
+ case BaseCommand::GET_CURSOR_RESPONSE:
+ return "GET_CURSOR_RESPONSE";
+ case BaseCommand::CREATE_CURSOR:
+ return "CREATE_CURSOR";
+ case BaseCommand::CREATE_CURSOR_RESPONSE:
+ return "CREATE_CURSOR_RESPONSE";
+ case BaseCommand::DELETE_CURSOR:
+ return "DELETE_CURSOR";
+ case BaseCommand::UPDATE_CURSOR:
+ return "UPDATE_CURSOR";
};
BOOST_THROW_EXCEPTION(std::logic_error("Invalid BaseCommand enumeration value"));
}
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-935557360
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 edited a comment on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 edited a comment on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-916579508
> @Jason918 Thanks for your contribution. For this PR, do we need to update docs?
>
> (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
@Anonymitaet
Sorry for that, the PR template says that `if you have committer privilege`, maybe I didn't get that right. I thought it's for committers ( with write access to the project ) to decide about the docs.
About user doc, This PR is not ready for end users to use this feature in a very convenient way. For example, some parameters is added in namespace policies, but it is not covered in pulsar-admin or REST admin interface.
I would like to add these admin tools and user docs in the following PRs.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-915182260
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-916828765
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-916927748
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-918827411
@codelipenghui @linlinnn Hi, can you please take a look at this PR?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a change in pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#discussion_r707931224
##########
File path: pulsar-client-api/src/main/java/org/apache/pulsar/client/api/CursorClient.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.api;
+
+import java.io.Closeable;
+import java.util.concurrent.CompletableFuture;
+import org.apache.pulsar.common.classification.InterfaceAudience;
+import org.apache.pulsar.common.classification.InterfaceStability;
+
+/**
+ * A {@link CursorClient} connects to a PersistentTopic and can manage cursor data of a persistent subscription.
+ * This is an interface that abstracts behavior of Pulsar's cursor client.
+ * This {@link CursorClient} is used by a readonly topic owner.
+ *
+ * @since 2.9.0
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Evolving
+public interface CursorClient extends Closeable {
Review comment:
@eolivelli Hi, I updated the doc in this PR doc. Please take a look.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-936193258
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a change in pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#discussion_r706099193
##########
File path: pulsar-client-api/src/main/java/org/apache/pulsar/client/api/CursorClient.java
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.api;
+
+import java.io.Closeable;
+import java.util.concurrent.CompletableFuture;
+import org.apache.pulsar.common.classification.InterfaceAudience;
+import org.apache.pulsar.common.classification.InterfaceStability;
+
+/**
+ * A {@link CursorClient} connects to a PersistentTopic and can manage cursor data of a persistent subscription.
+ * This is an interface that abstracts behavior of Pulsar's cursor client.
+ * This {@link CursorClient} is used by a readonly topic owner.
+ *
+ * @since 2.9.0
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Evolving
+public interface CursorClient extends Closeable {
Review comment:
@eolivelli Yes, it is a little different regarding to the complexity of implementation, but the main idea is the same.
And ...
This PIP doc is in wiki directory, actually I don't know how to update that. Any help?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#issuecomment-915135025
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on a change in pull request #11960: [PIP-63] Readonly-Topic-Ownership-Support
Posted by GitBox <gi...@apache.org>.
jiazhai commented on a change in pull request #11960:
URL: https://github.com/apache/pulsar/pull/11960#discussion_r713604485
##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java
##########
@@ -1850,6 +1831,10 @@ public void operationFailed(ManagedLedgerException exception) {
});
}
+ protected void persistPosition(MarkDeleteEntry mdEntry, VoidCallback callback) {
Review comment:
A minor question, why we need this wrapper?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org