You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "michaeljmarshall (via GitHub)" <gi...@apache.org> on 2023/02/13 20:13:43 UTC
[GitHub] [pulsar] michaeljmarshall opened a new pull request, #19506: [fix][broker] Make authentication refresh threadsafe
michaeljmarshall opened a new pull request, #19506:
URL: https://github.com/apache/pulsar/pull/19506
Fixes #19431
### Motivation
The `originalAuthState` and the `authState` are not thread safe members of the `ServerCnx` class. As such, we should only access them on the context's event loop thread.
### Modifications
* Replace scheduled task to check all connections for authentication refresh with a scheduled task per `ServerCnx` that will run on the appropriate event loop. This logic could theoretically be optimized to minimize scheduled tasks. However, I am following the keep alive implementation and making the assumption that the Netty scheduled tasks are not too expensive.
* Slightly refactor conditional logic in `refreshAuthenticationCredentials` method to remove the logic that it only short circuits when the `ServerCnx` is in `Failed` state. We should never call this method when the `isActive` flag has gone to `false`, and the only case when the state isn't `Connected` is when we recently failed due to an exception.
### Verifying this change
Added new test and make sure existing tests pass. Also, I added some new `assert` lines. These only run when the correct JVM arg is passed.
### Does this pull request potentially affect one of the following parts:
This is not a breaking change.
### Documentation
- [x] `doc-not-needed`
This is an internal improvement.
### Matching PR in forked repository
PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/28
--
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] lhotari merged pull request #19506: [fix][broker] Make authentication refresh threadsafe
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #19506:
URL: https://github.com/apache/pulsar/pull/19506
--
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] michaeljmarshall commented on pull request #19506: [fix][broker] Make authentication refresh threadsafe
Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #19506:
URL: https://github.com/apache/pulsar/pull/19506#issuecomment-1428868769
/pulsarbot rerun-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] codecov-commenter commented on pull request #19506: [fix][broker] Make authentication refresh threadsafe
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19506:
URL: https://github.com/apache/pulsar/pull/19506#issuecomment-1428957784
# [Codecov](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19506](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3425a88) into [master](https://codecov.io/gh/apache/pulsar/commit/016e7f0af997788d0514ca64e2c5c1bd9f506863?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (016e7f0) will **increase** coverage by `37.27%`.
> The diff coverage is `50.90%`.
[](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #19506 +/- ##
=============================================
+ Coverage 24.75% 62.03% +37.27%
- Complexity 288 25727 +25439
=============================================
Files 1579 1843 +264
Lines 121841 135152 +13311
Branches 13304 14860 +1556
=============================================
+ Hits 30164 83837 +53673
+ Misses 87240 43605 -43635
- Partials 4437 7710 +3273
```
| Flag | Coverage Δ | |
|---|---|---|
| inttests | `24.67% <5.45%> (-0.08%)` | :arrow_down: |
| systests | `25.26% <21.81%> (?)` | |
| unittests | `59.34% <50.90%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ulsar/broker/service/PulsarChannelInitializer.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1B1bHNhckNoYW5uZWxJbml0aWFsaXplci5qYXZh) | `90.00% <ø> (+25.52%)` | :arrow_up: |
| [...va/org/apache/pulsar/broker/service/ServerCnx.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1NlcnZlckNueC5qYXZh) | `55.10% <50.90%> (+23.44%)` | :arrow_up: |
| [...dbalance/extensions/ExtensibleLoadManagerImpl.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9sb2FkYmFsYW5jZS9leHRlbnNpb25zL0V4dGVuc2libGVMb2FkTWFuYWdlckltcGwuamF2YQ==) | `5.03% <0.00%> (-2.45%)` | :arrow_down: |
| [...er/systopic/NamespaceEventsSystemTopicFactory.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zeXN0b3BpYy9OYW1lc3BhY2VFdmVudHNTeXN0ZW1Ub3BpY0ZhY3RvcnkuamF2YQ==) | `71.42% <0.00%> (-1.91%)` | :arrow_down: |
| [...xtensions/channel/ServiceUnitStateChannelImpl.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9sb2FkYmFsYW5jZS9leHRlbnNpb25zL2NoYW5uZWwvU2VydmljZVVuaXRTdGF0ZUNoYW5uZWxJbXBsLmphdmE=) | `0.66% <0.00%> (-0.19%)` | :arrow_down: |
| [...in/java/org/apache/pulsar/common/api/AuthData.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9jb21tb24vYXBpL0F1dGhEYXRhLmphdmE=) | `71.42% <0.00%> (ø)` | |
| [.../apache/pulsar/broker/namespace/LookupOptions.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9uYW1lc3BhY2UvTG9va3VwT3B0aW9ucy5qYXZh) | `87.50% <0.00%> (ø)` | |
| [.../apache/pulsar/common/naming/SystemTopicNames.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9uYW1pbmcvU3lzdGVtVG9waWNOYW1lcy5qYXZh) | `55.55% <0.00%> (ø)` | |
| [...apache/pulsar/common/util/SafeCollectionUtils.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi91dGlsL1NhZmVDb2xsZWN0aW9uVXRpbHMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...pache/pulsar/common/configuration/BindAddress.java](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9jb21tb24vY29uZmlndXJhdGlvbi9CaW5kQWRkcmVzcy5qYXZh) | `22.22% <0.00%> (ø)` | |
| ... and [1388 more](https://codecov.io/gh/apache/pulsar/pull/19506?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
--
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