[improve][cli]Pulsar Dynamic Configuration Feature for Broker-level Configuration #12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gaozhangmin
changed the title
14 tasks
gaozhangmin
changed the title
gaozhangmin
changed the title
Co-authored-by: Andy Walker <andy@andy.dev>
Fixes apache#18466 ### Motivation There are two main goals in solving this issue: 1. Fix the unstable tests in `produceCommitTest`. 2. Prevent transaction timeouts created by other tests from affecting the `testTxnTimeoutAtTransactionMetadataStore` test during its execution. ### Modification 1. Change the message-sending method to synchronous. (fix `produceCommitTest`) 2. Increase the transaction timeout to 10 minutes (fix `testTxnTimeoutAtTransactionMetadataStore`).
…(And add compression and maxBatchSize for the load balance system topic) (apache#20087)
…egmentLastIndexTable & Make load operate asynchronous (apache#20086)
### Motivation Add a link to see existing PIP issues so you can select a number
Signed-off-by: tison <wander4096@gmail.com>
…t-forward cursor behavior after compaction (apache#20110) Master Issue: apache#16691 ### Motivation Raising a PR to implement: apache#16691 After the compaction, the cursor can fast-forward to the compacted horizon when a large number of messages are compacted before the next read. Hence, ServiceUnitStateCompactionStrategy also needs to cover this case. Currently, the existing and slow(their states are far behind) tableviews with ServiceUnitStateCompactionStrategy could not accept those compacted messages. In the load balance extension context, this means the ownership data could be inconsistent among brokers. ### Modifications This PR - fixes ServiceUnitStateCompactionStrategy to accept the state data if its version is bigger than the current version +1. - (minor fix) does not repeatedly update the replication_clusters in the policies when creating the system namespace. This update redundantly triggers ZK watchers when restarting brokers. - sets closeWithoutWaitingClientDisconnect=true, upon unload(following the same setting as the modular LM's)
Signed-off-by: nodece <nodeces@gmail.com>
…ist (apache#20132) PIP: apache#12105 and apache#19771 ### Motivation With the implementation of asynchronous authentication in PIP 97, I missed a case in the `AuthenticationProviderList` where we need to implement the `authenticateAsync` methods. This PR is necessary for making the `AuthenticationProviderToken` and the `AuthenticationProviderOpenID` work together, which is necessary for anyone transitioning to `AuthenticationProviderOpenID`. ### Modifications * Implement `AuthenticationListState#authenticateAsync` using a recursive algorithm that first attempts to authenticate the client using the current `authState` and then tries the remaining options. * Implement `AuthenticationProviderList#authenticateAsync` using a recursive algorithm that attempts each provider sequentially. * Add test to `AuthenticationProviderListTest` that exercises this method. It didn't technically fail previously, but it's worth adding. * Add test to `AuthenticationProviderOpenIDIntegrationTest` to cover the exact failures that were causing problems.
Signed-off-by: tison <wander4096@gmail.com>
Co-authored-by: druidliu <druidliu@tencent.com>
### Motivation The `AuthenticationProviderOpenID` error logs from the Kubernetes client are not very helpful in certain cases because we only get the error's message and not the error's response body. See kubernetes-client/java#2066 for details on the solution. Here is an example of a problematic error: ``` org.apache.pulsar.broker.authentication.AuthenticationProviderList - Authentication failed for auth provider class org.apache.pulsar.broker.authentication.oidc.AuthenticationProviderOpenID: javax.naming.AuthenticationException: Error retrieving OpenID Provider Metadata from Kubernetes API server: at org.apache.pulsar.broker.authentication.oidc.OpenIDProviderMetadataCache$1.onFailure(OpenIDProviderMetadataCache.java:174) ~[org.apache.pulsar-pulsar-broker-auth-oidc-3.0.0.jar:3.0.0] at io.kubernetes.client.openapi.ApiClient$1.onResponse(ApiClient.java:927) ~[io.kubernetes-client-java-api-17.0.2.jar:?] at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519) ~[com.squareup.okhttp3-okhttp-4.9.3.jar:?] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) ~[?:?] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) ~[?:?] at java.lang.Thread.run(Thread.java:833) ~[?:?] ``` When I enable debug logging out of the API Client, I can see: ``` INFO: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"forbidden: User \"system:serviceaccount:michael-test:superuser\" cannot get path \"/.well-known/openid-configuration/\"","reason":"Forbidden","details":{},"code":403} Apr 19, 2023 2:50:25 AM okhttp3.internal.platform.Platform log INFO: <-- END HTTP (246-byte body) 2023-04-19T02:50:25,832+0000 [pulsar-web-40-1] DEBUG ``` (Note: the solution to this problem is to update the `system:service-account-issuer-discovery` `ClusterRole` to include endpoints with trailing slashes. I created kubernetes/kubernetes#117455 to help solve the permission problem in kubernetes.) ### Modifications * Use both the message and the response body when converting a Kubernetes client error into a Pulsar Authentication error. ### Verifying this change This change is a trivial rework / code cleanup without any test coverage. ### Documentation - [x] `doc-not-needed` ### Matching PR in forked repository PR in forked repository: no need for a forked PR
…pache#20144) PIP: apache#19849 ### Motivation The new `KubernetesServiceAccountTokenAuthProvider` introduced by apache#19888 does not configure the authentication for download because there is an unnecessary check that the `getFunctionAuthenticationSpec` is not `null`. Given that we do not use this spec in creating the auth, it is unnecessary to use here. Further, when we construct the function's authentication, we do not have this null check. See: https://github.com/apache/pulsar/blob/ec102fb024a6ea2b195826778300f20e330dff06/pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java#L411-L417 ### Modifications * Update the `KubernetesRuntime` to add authentication params to the download command when provided. ### Verifying this change A new test is added. ### Documentation - [x] `doc-not-needed` ### Matching PR in forked repository PR in forked repository: Skipping for this trivial PR
…pache#20142) Fixes: apache#20066 ### Motivation In apache#20068 we changed the way that the `AuthorizationService` is implemented. I think we should change the `Consumer` and the `Producer` logic to call the correct `AuthorizationService` method. Given that our goal is to deprecate the `AuthorizationService` methods for `canProduce` and `canConsume`, this change helps us move in the right direction. ### Modifications * Update `Producer` and `Consumer` in broker to call the `AuthorizationService#allowTopicOperationAsync` method. ### Verifying this change This change is trivial. ### Documentation - [x] `doc-not-needed` ### Matching PR in forked repository PR in forked repository: Skipping PR as I ran tests locally.
### Motivation Removed unused reload4j define ### Does this pull request potentially affect one of the following parts: <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. --> *If the box was checked, please highlight the changes* - [ ] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The threading model - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [ ] The metrics - [ ] Anything that affects deployment ### Documentation <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. --> - [ ] `doc` <!-- Your PR contains doc changes. --> - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later --> - [x] `doc-not-needed` <!-- Your PR changes do not impact docs --> - [ ] `doc-complete` <!-- Docs have been already added --> ### Matching PR in forked repository PR in forked repository: hezhangjian#4 <!-- After opening this PR, the build in apache/pulsar will fail and instructions will be provided for opening a PR in the PR author's forked repository. apache/pulsar pull requests should be first tested in your own fork since the apache/pulsar CI based on GitHub Actions has constrained resources and quota. GitHub Actions provides separate quota for pull requests that are executed in a forked repository. The tests will be run in the forked repository until all PR review comments have been handled, the tests pass and the PR is approved by a reviewer. -->
…MultiConfig (apache#19436) Co-authored-by: tison <wander4096@gmail.com>
…e-auth-data` flag to update function/sink/source (apache#19450) Co-authored-by: tison <wander4096@gmail.com>
Co-authored-by: tison <wander4096@gmail.com>
…okerservice.getTopic() (apache#20569)
…2-21698 (apache#20579) Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
…pache#20599) Co-authored-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
…ache#20563) Co-authored-by: wangjinlong <wangjinlong@zhihu.com> ### Motivation Remove unused cache executor in PulsarService
…r creation/update (apache#20541)
…recate numWorkerThreadsForNonPersistentTopic in configuration (apache#20507) This is a pip to Introduce `topicOrderedExecutorThreadNum` to deprecate `numWorkerThreadsForNonPersistentTopic`
gaozhangmin
force-pushed
the
dynamic-configuration
branch
from
57988a3 to
7f3e408
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #xyz
Master Issue: #xyz
PIP: #xyz
Motivation
Modifications
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: