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 2022/08/15 10:55:13 UTC
[GitHub] [pulsar] Anonymitaet opened a new pull request, #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Anonymitaet opened a new pull request, #17099:
URL: https://github.com/apache/pulsar/pull/17099
Fix https://github.com/apache/pulsar/pull/13336#pullrequestreview-1068084883
I do not fix occurrences in `reference-configuration.md` because we're optimizing the workflow (trying to generate docs from code automatically for https://pulsar.apache.org/docs/next/reference-configuration)
--
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 #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #17099:
URL: https://github.com/apache/pulsar/pull/17099#issuecomment-1216207199
@BewareMyPower
Double check: I've updated `enableExposingBrokerEntryMetadataToClient` for these versions, is it correct?
<img width="424" alt="image" src="https://user-images.githubusercontent.com/50226895/184813611-b506cf67-449f-4ae5-908e-73705ceef7e4.png">
--
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 #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #17099:
URL: https://github.com/apache/pulsar/pull/17099#issuecomment-1217541558
ping @BewareMyPower again :-D
--
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 a diff in pull request #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #17099:
URL: https://github.com/apache/pulsar/pull/17099#discussion_r947561789
##########
site2/website/versioned_docs/version-2.1.1-incubating/developing-binary-protocol.md:
##########
@@ -68,7 +68,7 @@ If you want to use broker entry metadata for **consumers**:
1. Use the client protocol version [18 or later](https://github.com/apache/pulsar/blob/ca37e67211feda4f7e0984e6414e707f1c1dfd07/pulsar-common/src/main/proto/PulsarApi.proto#L259).
-2. Configure the [`brokerEntryMetadataInterceptors`](reference-configuration.md#broker) parameter and set the [`enableExposingBrokerEntryMetadataToClient`](reference-configuration.md#broker) parameter to `true` in the `broker.conf` file.
Review Comment:
`brokerEntryMetadataInterceptors` was introduced since 2.8.0, but `enableExposingBrokerEntryMetadataToClient` was introduced since 2.10.0.
--
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 #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #17099:
URL: https://github.com/apache/pulsar/pull/17099#issuecomment-1217696364
@RobertIndie @BewareMyPower thanks for your suggestions!
I've updated this PR based on your comments. The changes are summarized below, PTAL.
1. Keep the following content for 2.8.x and 2.9.x.
```
## Broker entry metadata
Broker entry metadata is stored alongside the message metadata as a serialized protobuf message.
It is created by the broker when the message arrived at the broker and passed without changes to the consumer if configured.
| Field | Required or optional | Description |
|:-------------------|:----------------|:------------------------------------------------------------------------------------------------------------------------------|
| `broker_timestamp` | Optional | The timestamp when a message arrived at the broker (`id est` as the number of milliseconds since January 1st, 1970 in UTC) |
| `index` | Optional | The index of the message. It is assigned by the broker.
If you want to use broker entry metadata for **brokers**, configure the [`brokerEntryMetadataInterceptors`](reference-configuration.md#broker) parameter in the `broker.conf` file.
```
2. Keep the following content for 2.10.x.
```
## Broker entry metadata
Broker entry metadata is stored alongside the message metadata as a serialized protobuf message.
It is created by the broker when the message arrived at the broker and passed without changes to the consumer if configured.
| Field | Required or optional | Description |
|:-------------------|:----------------|:------------------------------------------------------------------------------------------------------------------------------|
| `broker_timestamp` | Optional | The timestamp when a message arrived at the broker (`id est` as the number of milliseconds since January 1st, 1970 in UTC) |
| `index` | Optional | The index of the message. It is assigned by the broker.
If you want to use broker entry metadata for **brokers**, configure the [`brokerEntryMetadataInterceptors`](reference-configuration.md#broker) parameter in the `broker.conf` file.
If you want to use broker entry metadata for **consumers**:
1. Use the client protocol version [18 or later](https://github.com/apache/pulsar/blob/ca37e67211feda4f7e0984e6414e707f1c1dfd07/pulsar-common/src/main/proto/PulsarApi.proto#L259).
2. Configure the [`brokerEntryMetadataInterceptors`](reference-configuration.md#broker) parameter and set the [`exposingBrokerEntryMetadataToClientEnabled`](reference-configuration-broker.md#exposingbrokerentrymetadatatoclientenabled) parameter to `true` in the `broker.conf` file.
```
3. Remove the following content for versions prior to 2.8.0.
```
## Broker entry metadata
Broker entry metadata is stored alongside the message metadata as a serialized protobuf message.
It is created by the broker when the message arrived at the broker and passed without changes to the consumer if configured.
| Field | Required or optional | Description |
|:-------------------|:----------------|:------------------------------------------------------------------------------------------------------------------------------|
| `broker_timestamp` | Optional | The timestamp when a message arrived at the broker (`id est` as the number of milliseconds since January 1st, 1970 in UTC) |
| `index` | Optional | The index of the message. It is assigned by the broker.
If you want to use broker entry metadata for **brokers**, configure the [`brokerEntryMetadataInterceptors`](reference-configuration.md#broker) parameter in the `broker.conf` file.
If you want to use broker entry metadata for **consumers**:
1. Use the client protocol version [18 or later](https://github.com/apache/pulsar/blob/ca37e67211feda4f7e0984e6414e707f1c1dfd07/pulsar-common/src/main/proto/PulsarApi.proto#L259).
2. Configure the [`brokerEntryMetadataInterceptors`](reference-configuration.md#broker) parameter and set the [`exposingBrokerEntryMetadataToClientEnabled`](reference-configuration-broker.md#exposingbrokerentrymetadatatoclientenabled) parameter to `true` in the `broker.conf` file.
```
4. Ignore updating the versions in red and only updating versions in green since red are deprecated. Details see [Update versioned docs](https://docs.google.com/document/d/1-1uJyd1k9_h56xiiVRVOnrLcCnTmg9n7SrHhNVNEEi4/edit#bookmark=id.q5m2r5pimdi6).
<img width="235" alt="image" src="https://user-images.githubusercontent.com/50226895/185074601-218a45f3-e74d-44e0-8fd4-e4104dd4bff8.png">
cc @momo-jun
--
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 #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #17099:
URL: https://github.com/apache/pulsar/pull/17099#issuecomment-1217831356
I found another problem when I reviewed this PR but it's not related. I found some versioned docs contain both `develop-binary-protocol.md` and `developing-binary-protocol.md`. I compared these two files in `version-2.9.1` directory (after applying changes of this PR), they are nearly the same.
```diff
2c2
< id: develop-binary-protocol
---
> id: developing-binary-protocol
5c5
< original_id: develop-binary-protocol
---
> original_id: developing-binary-protocol
400a401,425
> * `properties` → *(optional)* Reserved configuration items
> * `txnid_most_bits` → *(optional)* Same as Transaction Coordinator ID, `txnid_most_bits` and `txnid_least_bits`
> uniquely identify a transaction.
> * `txnid_least_bits` → *(optional)* The ID of the transaction opened in a transaction coordinator,
> `txnid_most_bits` and `txnid_least_bits`uniquely identify a transaction.
> * `request_id` → *(optional)* ID for handling response and timeout.
>
>
> ##### Command AckResponse
>
> An `AckResponse` is the broker’s response to acknowledge a request sent by the client. It contains the `consumer_id` sent in the request.
> If a transaction is used, it contains both the Transaction ID and the Request ID that are sent in the request. The client finishes the specific request according to the Request ID. If the `error` field is set, it indicates that the request has failed.
>
> An example of `AckResponse` with redirection:
>
> ```protobuf
>
> message CommandAckResponse {
> "consumer_id" : 1,
> "txnid_least_bits" = 0,
> "txnid_most_bits" = 1,
> "request_id" = 5
> }
>
> ```
461c486
< Lookup can be done with a REST call as described in the [admin API](admin-api-topics.md#look-up-topics-owner-broker)
---
> Lookup can be done with a REST call as described in the [admin API](admin-api-topics.md#lookup-of-topic)
```
Could we delete the develop-binary-protocol.md?
--
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] RobertIndie commented on a diff in pull request #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Posted by GitBox <gi...@apache.org>.
RobertIndie commented on code in PR #17099:
URL: https://github.com/apache/pulsar/pull/17099#discussion_r945686350
##########
site2/website/versioned_docs/version-2.1.1-incubating/developing-binary-protocol.md:
##########
@@ -68,7 +68,7 @@ If you want to use broker entry metadata for **consumers**:
1. Use the client protocol version [18 or later](https://github.com/apache/pulsar/blob/ca37e67211feda4f7e0984e6414e707f1c1dfd07/pulsar-common/src/main/proto/PulsarApi.proto#L259).
-2. Configure the [`brokerEntryMetadataInterceptors`](reference-configuration.md#broker) parameter and set the [`enableExposingBrokerEntryMetadataToClient`](reference-configuration.md#broker) parameter to `true` in the `broker.conf` file.
Review Comment:
The brokerEntryMetadataInterceptors should not appear before V2.10.0. It's the same with https://github.com/apache/pulsar/pull/17004
--
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 #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #17099:
URL: https://github.com/apache/pulsar/pull/17099#issuecomment-1219110425
@BewareMyPower thank you! Suggest keeping it since there might be potential issues if it's removed.
Besides, keeping it does not cause trouble for doc maintainers since there is only `developing-binary-protocol.md` (no `develop-binary-protocol.md`) in `version-2.8.x`, `version-2.9.x`, `version-2.10.x`, `master`, and versions forward.
--
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 merged pull request #17099: [fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled
Posted by GitBox <gi...@apache.org>.
Anonymitaet merged PR #17099:
URL: https://github.com/apache/pulsar/pull/17099
--
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