You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by "hangc0276 (via GitHub)" <gi...@apache.org> on 2023/02/26 03:25:36 UTC
[GitHub] [bookkeeper] hangc0276 opened a new pull request, #3809: Speed up shutdown in test cases
hangc0276 opened a new pull request, #3809:
URL: https://github.com/apache/bookkeeper/pull/3809
Fix #3808
### Motivation
When the tests run in the following configurations
- The BookKeeper cluster has multiple bookies (more than 2 bookies)
- Enabled auto-recovery
When we shut down the bookkeeper cluster, it will take more than 30s. The root cause is that we don't shut down **all** the auto-recovery service before we shut down the bookies.
https://github.com/apache/bookkeeper/blob/80d3aac30f69d2a10f06dca4f9c2a62b8d01220a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java#L949-L972
When we shut down one bookie, the auditor may change from one bookie to another and start to do all the checking tasks, which will block the following shutdown process. The shutdown process will wait at most 30s and then will call `executor.shutdownNow()` to shut down all the tasks.
https://github.com/apache/bookkeeper/blob/80d3aac30f69d2a10f06dca4f9c2a62b8d01220a/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java#L627-L632
The logs of the slow tests.
```
2023-02-26T09:43:07,576 - INFO - [Time-limited test:AutoRecoveryMain@147] - Shutting down auto recovery: 0
2023-02-26T09:43:07,577 - INFO - [Time-limited test:AutoRecoveryMain@151] - Shutting down AutoRecovery
2023-02-26T09:43:07,578 - INFO - [Time-limited test:Auditor@626] - Shutting down auditor
2023-02-26T09:43:07,578 - INFO - [AuditorElector-127.0.0.1:62826:ZkLedgerAuditorManager@176] - Shutting down AuditorElector
2023-02-26T09:43:07,586 - INFO - [AuditorElector-127.0.0.1:62830:Auditor@203] - AuthProvider used by the Auditor is null
2023-02-26T09:43:07,588 - INFO - [AuditorElector-127.0.0.1:62830:ZkLedgerUnderreplicationManager@823] - lostBookieRecoveryDelay Znode is already present, so using existing lostBookieRecoveryDelay Znode value
2023-02-26T09:43:07,588 - INFO - [AuditorElector-127.0.0.1:62830:Auditor@210] - Valid lostBookieRecoveryDelay zNode is available, so not creating lostBookieRecoveryDelay zNode as part of Auditor initialization
2023-02-26T09:43:07,588 - INFO - [AuditorElector-127.0.0.1:62830:Auditor@380] - I'm starting as Auditor Bookie. ID: 127.0.0.1:62830
2023-02-26T09:43:07,590 - INFO - [AuditorElector-127.0.0.1:62830:Auditor@431] - Auditor periodic bookie checking enabled 'auditorPeriodicBookieCheckInterval' 86400 seconds
2023-02-26T09:43:07,590 - INFO - [AuditorElector-127.0.0.1:62830:Auditor@441] - Auditor periodic ledger checking enabled 'auditorPeriodicCheckInterval' 604800 seconds
2023-02-26T09:43:07,591 - INFO - [AuditorElector-127.0.0.1:62830:Auditor@470] - checkAllLedgers scheduling info. checkAllLedgersLastExecutedCTime: 1677375779900 durationSinceLastExecutionInSecs: 7 initialDelay: 604793 interval: 604800
2023-02-26T09:43:07,591 - INFO - [AuditorBookie-127.0.0.1:62830:AuditorBookieCheckTask@119] - Starting auditBookies
2023-02-26T09:43:07,591 - INFO - [AuditorElector-127.0.0.1:62830:Auditor@521] - Periodic placementPolicy check disabled
2023-02-26T09:43:07,591 - INFO - [AuditorElector-127.0.0.1:62830:Auditor@529] - Periodic replicas check disabled
2023-02-26T09:43:07,595 - INFO - [AuditorBookie-127.0.0.1:62830:AuditorBookieCheckTask@156] - Completed auditBookies
2023-02-26T09:43:07,850 - INFO - [AutoRecoveryDeathWatcher-62826:AutoRecoveryMain$AutoRecoveryDeathWatcher@235] - AutoRecoveryDeathWatcher noticed the AutoRecovery is not running any more,exiting the watch loop!
2023-02-26T09:43:07,850 - INFO - [AutoRecoveryDeathWatcher-62826:AutoRecoveryMain$AutoRecoveryDeathWatcher@219] - AutoRecoveryDeathWatcher exited loop due to uncaught exception from thread AutoRecoveryDeathWatcher-62826
java.lang.RuntimeException: AutoRecovery is not running any more
at org.apache.bookkeeper.replication.AutoRecoveryMain$AutoRecoveryDeathWatcher.run(AutoRecoveryMain.java:245) ~[classes/:?]
2023-02-26T09:43:07,851 - INFO - [AutoRecoveryDeathWatcher-62826:AutoRecoveryMain@147] - Shutting down auto recovery: 0
2023-02-26T09:43:37,501 - WARN - [Time-limited test:Auditor@630] - Executor not shutting down, interrupting
2023-02-26T09:43:37,503 - INFO - [Time-limited test:AuditorCheckAllLedgersTask@130] - Shutting down AuditorCheckAllLedgersTask
2023-02-26T09:43:37,503 - INFO - [Time-limited test:ReplicationWorker@728] - Shutting down replication worker
2023-02-26T09:43:37,503 - INFO - [Time-limited test:ReplicationWorker@736] - Shutting down ReplicationWorker
```
### Modifications
Shutdown the auto-recovery service before shutting down bookies in the test cluster.
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3809: Speed up shutdown in test cases
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3809:
URL: https://github.com/apache/bookkeeper/pull/3809#issuecomment-1446098543
After applying this change, the `BookKeeperAdminTest` cost time was reduced from `1,462.832` s to `294.611` s
```
[INFO] Running org.apache.bookkeeper.client.BookKeeperAdminTest
Error: WARNING: An illegal reflective access operation has occurred
Error: WARNING: Illegal reflective access by org.apache.bookkeeper.util.NativeIO (file:/home/runner/work/bookkeeper/bookkeeper/bookkeeper-server/target/classes/) to field java.io.FileDescriptor.fd
Error: WARNING: Please consider reporting this to the maintainers of org.apache.bookkeeper.util.NativeIO
Error: WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
Error: WARNING: All illegal access operations will be denied in a future release
[INFO] Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 294.611 s - in org.apache.bookkeeper.client.BookKeeperAdminTest
```
The `BookKeeper CI / Client Tests` total cost time was reduced from `53m 52s` to `24m 30s`, and reduced by 54.7% of the time.
**Before this PR:**
https://github.com/apache/bookkeeper/actions/runs/4279252006/jobs/7449831357
Running `BookKeeper CI / Client Tests` cost `53m 52s`
**After this PR:**
https://github.com/apache/bookkeeper/actions/runs/4280984915/jobs/7453515098
Running `BookKeeper CI / Client Tests` cost `24m 30s`
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] codecov-commenter commented on pull request #3809: Speed up shutdown in test cases
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3809:
URL: https://github.com/apache/bookkeeper/pull/3809#issuecomment-1445261225
# [Codecov](https://codecov.io/gh/apache/bookkeeper/pull/3809?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 [#3809](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (33291e0) into [master](https://codecov.io/gh/apache/bookkeeper/commit/80d3aac30f69d2a10f06dca4f9c2a62b8d01220a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (80d3aac) will **decrease** coverage by `39.46%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #3809 +/- ##
=============================================
- Coverage 60.47% 21.01% -39.46%
+ Complexity 5847 2006 -3841
=============================================
Files 473 473
Lines 40933 40933
Branches 5235 5235
=============================================
- Hits 24756 8604 -16152
- Misses 13976 31052 +17076
+ Partials 2201 1277 -924
```
| Flag | Coverage Δ | |
|---|---|---|
| bookie | `?` | |
| remaining | `?` | |
| replication | `?` | |
| tls | `21.01% <ø> (+0.02%)` | :arrow_up: |
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/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../java/org/apache/bookkeeper/util/SubTreeCache.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvdXRpbC9TdWJUcmVlQ2FjaGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/bookkeeper/proto/BookieClient.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvcHJvdG8vQm9va2llQ2xpZW50LmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/bookkeeper/bookie/ReadOnlyFileInfo.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvYm9va2llL1JlYWRPbmx5RmlsZUluZm8uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/bookkeeper/bookie/datainteg/Events.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvYm9va2llL2RhdGFpbnRlZy9FdmVudHMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rg/apache/bookkeeper/client/api/CreateBuilder.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvY2xpZW50L2FwaS9DcmVhdGVCdWlsZGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rg/apache/bookkeeper/metastore/MetastoreTable.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWV0YXN0b3JlL01ldGFzdG9yZVRhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/bookkeeper/bookie/storage/EntryLogger.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvYm9va2llL3N0b3JhZ2UvRW50cnlMb2dnZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/bookkeeper/meta/UnderreplicatedLedger.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWV0YS9VbmRlcnJlcGxpY2F0ZWRMZWRnZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...che/bookkeeper/bookie/storage/ldb/LongWrapper.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvYm9va2llL3N0b3JhZ2UvbGRiL0xvbmdXcmFwcGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/bookkeeper/server/conf/BookieConfiguration.java](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Ym9va2tlZXBlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvc2VydmVyL2NvbmYvQm9va2llQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [338 more](https://codecov.io/gh/apache/bookkeeper/pull/3809?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #3809: Speed up shutdown in test cases
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli commented on PR #3809:
URL: https://github.com/apache/bookkeeper/pull/3809#issuecomment-1445334522
There are many failure on CI
While I support this patch, I think we should understand why there are so many failure
I would like to not add new flakyness
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3809: Speed up shutdown in test cases
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3809:
URL: https://github.com/apache/bookkeeper/pull/3809#issuecomment-1446088148
> There are many failure on CI
>
> While I support this patch, I think we should understand why there are so many failure
>
> I would like to not add new flakyness
@eolivelli The root cause is that I stop the auto recovery service when one bookie shutdown. There are many tests related to auto-recovery will be failed.
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] merlimat merged pull request #3809: Speed up shutdown in test cases
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #3809:
URL: https://github.com/apache/bookkeeper/pull/3809
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3809: Speed up shutdown in test cases
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3809:
URL: https://github.com/apache/bookkeeper/pull/3809#issuecomment-1445259474
Due to the shutdown logic in `BookKeeperClusterTestCase` having been changed, and this change being hard to cherry-pick, I will push another two PRs for branch-4.14 and branch-4.15 after this PR is merged.
Time cost in branch-4.15. https://github.com/apache/bookkeeper/actions/runs/4238366553/jobs/7365358518
```
2023-02-22T01:33:27.7354051Z [INFO] Running org.apache.bookkeeper.client.BookKeeperAdminTest
2023-02-22T01:34:47.8626205Z [ERROR] WARNING: An illegal reflective access operation has occurred
2023-02-22T01:34:47.8627441Z [ERROR] WARNING: Illegal reflective access by org.apache.bookkeeper.proto.checksum.DirectMemoryCRC32Digest (file:/home/runner/work/bookkeeper/bookkeeper/bookkeeper-server/target/classes/) to method java.util.zip.CRC32.updateByteBuffer(int,long,int,int)
2023-02-22T01:34:47.8628403Z [ERROR] WARNING: Please consider reporting this to the maintainers of org.apache.bookkeeper.proto.checksum.DirectMemoryCRC32Digest
2023-02-22T01:34:47.8629219Z [ERROR] WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
2023-02-22T01:34:47.8629650Z [ERROR] WARNING: All illegal access operations will be denied in a future release
2023-02-22T01:46:48.6604300Z [INFO] Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 800.923 s - in org.apache.bookkeeper.client.BookKeeperAdminTest
```
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org