Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove direct ZK access for Auditor #2842

Merged
merged 4 commits into from
Oct 23, 2021
Merged

Remove direct ZK access for Auditor #2842

merged 4 commits into from
Oct 23, 2021

Conversation

merlimat
Copy link
Contributor

Motivation

BK Auditor shouldn't have direct access to ZooKeeper. Instead the logic for electing the auditor should be encapsulated
into an interface, like all the other metadata accesses.

@merlimat merlimat added this to the 4.15.0 milestone Oct 22, 2021
@merlimat merlimat self-assigned this Oct 22, 2021
@merlimat merlimat merged commit 746f9f6 into apache:master Oct 23, 2021
@merlimat merlimat deleted the auditor-remove-zookeeper branch October 23, 2021 04:21
merlimat added a commit that referenced this pull request Oct 23, 2021
@merlimat
Remove direct ZK access for Auditor (#2842)
c01ed32
@hezhangjian hezhangjian mentioned this pull request Oct 25, 2021
Merged
hsaputra pushed a commit that referenced this pull request Oct 29, 2021
@nicoloboschi
Gradle build - some submodules are skipped
98ddf81 
### Motivation

In order to complete migration to Gradle we must build all the subprojects.

### Changes

- Enabled `sh` integration tests with gradle, located in `tests/scripts/src/test/bash/gradle`
- Added these modules to the build
    - `bookkeeper-http:servlet-http-server` 
    - `metadata-drivers:etcd`
    - `tests:backward-compat:*`
    - `tests:shaded:*`
    - `stream:bk-grpc-name-resolver`
- DL shading process is now performed (before it didn't build any jar)
- Groovy tests (`tests:backward-compat:*`) now are triggered by the build/tests itself; with Maven, there is a "runner" project (`tests/integration-tests-base-groovy`); in Gradle is useless so it is skipped


### Test

- Both `bin/bookkeper standalone` and `bin/bookkeper_gradle standalone` work locally
- Tests are passing locally  

Master Issue: #2849



Reviewers: Henry Saputra <[email protected]>, Prashant Kumar <None>

This closes #2850 from nicoloboschi/fix/2849/gradle and squashes the following commits:

00b49f4 [Nicolò Boschi] Fix common_gradle.sh regex
bd739fd [Nicolò Boschi] fix sh tests
43230ba [Nicolò Boschi] revert sh files. Avoid to modify maven files, create gradle versions to faciltate migration
d1f95e4 [Nicolò Boschi] fix shaded deps
bcab40d [Nicolò Boschi] fix build
5fd0341 [Nicolò Boschi] fix build
0082e0e [Nicolò Boschi] fix build
2c32ac1 [Nicolò Boschi] fixes
3bc0b26 [Nicolò Boschi] bookkeeper-server-shaded-tests
ba89132 [Nicolò Boschi] shaded tests
6d39e33 [Nicolò Boschi] sh tests
e0032bc [Nicolò Boschi] actually run arquillian groovy tests
08dcc39 [Nicolò Boschi] backwards
2361f79 [Nicolò Boschi] hierarchical-ledger-manager
8388e11 [Nicolò Boschi] current-server-old-clients
6a24344 [Nicolò Boschi] bc-non-fips
2faca01 [Nicolò Boschi] bk-grpc-name-resolver
991bc11 [Nicolò Boschi] servlet-http-server
675ef7b [Nicolò Boschi] etcd
b1d5e14 [ZhangJian He] A empty implement in EtcdLedgerManagerFactory to let the project can compile (#2845)
bd5c50b [shustsud] Add error handling to readLedgerMetadata in over-replicated ledger GC (#2844)
746f9f6 [Matteo Merli] Remove direct ZK access for Auditor (#2842)
4117200 [ZhangJian He] the compare should be >= instead of > (#2782)
14ef56f [Prashant Kumar] BookieId can not be cast to BookieSocketAddress (#2843)
e10f3fe [ZhangJian He] Forget to close preAllocator log on shutdown (#2819)
53954ca [shustsud] Add ensemble check to over-replicated ledger GC (#2813)
919fdd3 [Prashant Kumar] Issue:2840 Create bookie shellscript for gradle (#2841)
031d168 [gaozhangmin] fix-npe-when-pulsar-ZkBookieRackAffinityMapping-getBookieAddressResolver (#2788)
3dd671c [Prashant Kumar] Migrate bookkeepr-server:test to gradle run unit tests excepts org.apache.bookkeeper.bookie. org.apache.bookkeeper.client org.apache.bookkeeper.replication org.apache.bookkeeper.tls. (#2812)
f6903b8 [Jack Vanlightly] BP-44 USE metrics
a4afaa4 [Matteo Merli] Eliminate direct ZK access in ScanAndCompareGarbageCollector (#2833)
a9b576d [Yunze Xu] Release semaphore when addEntry accepts the same entries (#2832)
148bf22 [Yun Tang] Ensure to release cache during KeyValueStorageRocksDB#closec (#2821)
4dc4260 [gaozhangmin] Heap memory leak problem when ledger replication failed (#2794)
a522fa3 [Raúl Gracia] Issue 2815: Upgrade to log4j2 to get rid of CVE-2019-17571 (#2816)
0465052 [Nicolò Boschi] Upgrade httpclient from 4.5.5 to 4.5.13 (#2793)
594a056 [Raúl Gracia] Issue 2795: Bookkeeper upgrade using Bookie ID may fail due to cookie mismatch (#2796)
354cf37 [Raúl Gracia] Upgraded dependencies with CVEs (#2792)
e413c70 [Raúl Gracia] Issue 2728: Entry Log GC may get blocked when using entryLogPerLedgerEnabled option (#2779)
883231e [pradeepbn] Building bookkeeper with gradle on java11
@wenbingshen wenbingshen mentioned this pull request Sep 26, 2022
Merged
zymap pushed a commit that referenced this pull request Oct 19, 2022
@wenbingshen
fix close ledgerAuditorManager repeatedly (#3503)
dbec4e9 
### Motivation
this introduced in #2842, the `ledgerAuditorManager` will be close repeatedly.

```java
    private void submitShutdownTask() {
        executor.submit(new Runnable() {
                @OverRide
                public void run() {
                    if (!running.compareAndSet(true, false)) {
                        return;
                    }

                    try {
                        ledgerAuditorManager.close();   <- here has already closed LAM.
                    } catch (InterruptedException ie) {
                        Thread.currentThread().interrupt();
                        LOG.warn("InterruptedException while closing ledger auditor manager", ie);
                    } catch (Exception ke) {
                        LOG.error("Exception while closing ledger auditor manager", ke);
                    }
                }
            });
    }
```
zymap pushed a commit that referenced this pull request Oct 26, 2022
@wenbingshen @zymap
fix close ledgerAuditorManager repeatedly (#3503)
5a4f927 
### Motivation
this introduced in #2842, the `ledgerAuditorManager` will be close repeatedly.

```java
    private void submitShutdownTask() {
        executor.submit(new Runnable() {
                @OverRide
                public void run() {
                    if (!running.compareAndSet(true, false)) {
                        return;
                    }

                    try {
                        ledgerAuditorManager.close();   <- here has already closed LAM.
                    } catch (InterruptedException ie) {
                        Thread.currentThread().interrupt();
                        LOG.warn("InterruptedException while closing ledger auditor manager", ie);
                    } catch (Exception ke) {
                        LOG.error("Exception while closing ledger auditor manager", ke);
                    }
                }
            });
    }
```

(cherry picked from commit dbec4e9)
hangc0276 pushed a commit to hangc0276/bookkeeper that referenced this pull request Nov 7, 2022
@wenbingshen @hangc0276
fix close ledgerAuditorManager repeatedly (apache#3503)
7a496c1 
### Motivation
this introduced in apache#2842, the `ledgerAuditorManager` will be close repeatedly.

```java
    private void submitShutdownTask() {
        executor.submit(new Runnable() {
                @OverRide
                public void run() {
                    if (!running.compareAndSet(true, false)) {
                        return;
                    }

                    try {
                        ledgerAuditorManager.close();   <- here has already closed LAM.
                    } catch (InterruptedException ie) {
                        Thread.currentThread().interrupt();
                        LOG.warn("InterruptedException while closing ledger auditor manager", ie);
                    } catch (Exception ke) {
                        LOG.error("Exception while closing ledger auditor manager", ke);
                    }
                }
            });
    }
```

(cherry picked from commit dbec4e9)
hangc0276 pushed a commit to hangc0276/bookkeeper that referenced this pull request Nov 7, 2022
@wenbingshen @hangc0276
fix close ledgerAuditorManager repeatedly (apache#3503)
224254b 
### Motivation
this introduced in apache#2842, the `ledgerAuditorManager` will be close repeatedly.

```java
    private void submitShutdownTask() {
        executor.submit(new Runnable() {
                @OverRide
                public void run() {
                    if (!running.compareAndSet(true, false)) {
                        return;
                    }

                    try {
                        ledgerAuditorManager.close();   <- here has already closed LAM.
                    } catch (InterruptedException ie) {
                        Thread.currentThread().interrupt();
                        LOG.warn("InterruptedException while closing ledger auditor manager", ie);
                    } catch (Exception ke) {
                        LOG.error("Exception while closing ledger auditor manager", ke);
                    }
                }
            });
    }
```

(cherry picked from commit dbec4e9)
nicoloboschi pushed a commit to datastax/bookkeeper that referenced this pull request Jan 11, 2023
@wenbingshen @nicoloboschi
fix close ledgerAuditorManager repeatedly (apache#3503)
f6d6405 
### Motivation
this introduced in apache#2842, the `ledgerAuditorManager` will be close repeatedly.

```java
    private void submitShutdownTask() {
        executor.submit(new Runnable() {
                @OverRide
                public void run() {
                    if (!running.compareAndSet(true, false)) {
                        return;
                    }

                    try {
                        ledgerAuditorManager.close();   <- here has already closed LAM.
                    } catch (InterruptedException ie) {
                        Thread.currentThread().interrupt();
                        LOG.warn("InterruptedException while closing ledger auditor manager", ie);
                    } catch (Exception ke) {
                        LOG.error("Exception while closing ledger auditor manager", ke);
                    }
                }
            });
    }
```

(cherry picked from commit dbec4e9)
(cherry picked from commit 224254b)
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
@merlimat
Remove direct ZK access for Auditor (apache#2842)
51d5689 
* Remove direct ZK access for Auditor

* Fixed unused imports

* Fixed checkstyle

* Fixed checkstyle in tests
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
@nicoloboschi
Gradle build - some submodules are skipped
ff821cb 
### Motivation

In order to complete migration to Gradle we must build all the subprojects.

### Changes

- Enabled `sh` integration tests with gradle, located in `tests/scripts/src/test/bash/gradle`
- Added these modules to the build
    - `bookkeeper-http:servlet-http-server` 
    - `metadata-drivers:etcd`
    - `tests:backward-compat:*`
    - `tests:shaded:*`
    - `stream:bk-grpc-name-resolver`
- DL shading process is now performed (before it didn't build any jar)
- Groovy tests (`tests:backward-compat:*`) now are triggered by the build/tests itself; with Maven, there is a "runner" project (`tests/integration-tests-base-groovy`); in Gradle is useless so it is skipped


### Test

- Both `bin/bookkeper standalone` and `bin/bookkeper_gradle standalone` work locally
- Tests are passing locally  

Master Issue: apache#2849



Reviewers: Henry Saputra <[email protected]>, Prashant Kumar <None>

This closes apache#2850 from nicoloboschi/fix/2849/gradle and squashes the following commits:

00b49f4 [Nicolò Boschi] Fix common_gradle.sh regex
bd739fd [Nicolò Boschi] fix sh tests
43230ba [Nicolò Boschi] revert sh files. Avoid to modify maven files, create gradle versions to faciltate migration
d1f95e4 [Nicolò Boschi] fix shaded deps
bcab40d [Nicolò Boschi] fix build
5fd0341 [Nicolò Boschi] fix build
0082e0e [Nicolò Boschi] fix build
2c32ac1 [Nicolò Boschi] fixes
3bc0b26 [Nicolò Boschi] bookkeeper-server-shaded-tests
ba89132 [Nicolò Boschi] shaded tests
6d39e33 [Nicolò Boschi] sh tests
e0032bc [Nicolò Boschi] actually run arquillian groovy tests
08dcc39 [Nicolò Boschi] backwards
2361f79 [Nicolò Boschi] hierarchical-ledger-manager
8388e11 [Nicolò Boschi] current-server-old-clients
6a24344 [Nicolò Boschi] bc-non-fips
2faca01 [Nicolò Boschi] bk-grpc-name-resolver
991bc11 [Nicolò Boschi] servlet-http-server
675ef7b [Nicolò Boschi] etcd
b1d5e14 [ZhangJian He] A empty implement in EtcdLedgerManagerFactory to let the project can compile (apache#2845)
bd5c50b [shustsud] Add error handling to readLedgerMetadata in over-replicated ledger GC (apache#2844)
746f9f6 [Matteo Merli] Remove direct ZK access for Auditor (apache#2842)
4117200 [ZhangJian He] the compare should be >= instead of > (apache#2782)
14ef56f [Prashant Kumar] BookieId can not be cast to BookieSocketAddress (apache#2843)
e10f3fe [ZhangJian He] Forget to close preAllocator log on shutdown (apache#2819)
53954ca [shustsud] Add ensemble check to over-replicated ledger GC (apache#2813)
919fdd3 [Prashant Kumar] Issue:2840 Create bookie shellscript for gradle (apache#2841)
031d168 [gaozhangmin] fix-npe-when-pulsar-ZkBookieRackAffinityMapping-getBookieAddressResolver (apache#2788)
3dd671c [Prashant Kumar] Migrate bookkeepr-server:test to gradle run unit tests excepts org.apache.bookkeeper.bookie. org.apache.bookkeeper.client org.apache.bookkeeper.replication org.apache.bookkeeper.tls. (apache#2812)
f6903b8 [Jack Vanlightly] BP-44 USE metrics
a4afaa4 [Matteo Merli] Eliminate direct ZK access in ScanAndCompareGarbageCollector (apache#2833)
a9b576d [Yunze Xu] Release semaphore when addEntry accepts the same entries (apache#2832)
148bf22 [Yun Tang] Ensure to release cache during KeyValueStorageRocksDB#closec (apache#2821)
4dc4260 [gaozhangmin] Heap memory leak problem when ledger replication failed (apache#2794)
a522fa3 [Raúl Gracia] Issue 2815: Upgrade to log4j2 to get rid of CVE-2019-17571 (apache#2816)
0465052 [Nicolò Boschi] Upgrade httpclient from 4.5.5 to 4.5.13 (apache#2793)
594a056 [Raúl Gracia] Issue 2795: Bookkeeper upgrade using Bookie ID may fail due to cookie mismatch (apache#2796)
354cf37 [Raúl Gracia] Upgraded dependencies with CVEs (apache#2792)
e413c70 [Raúl Gracia] Issue 2728: Entry Log GC may get blocked when using entryLogPerLedgerEnabled option (apache#2779)
883231e [pradeepbn] Building bookkeeper with gradle on java11
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
@wenbingshen
fix close ledgerAuditorManager repeatedly (apache#3503)
df5d27d 
### Motivation
this introduced in apache#2842, the `ledgerAuditorManager` will be close repeatedly.

```java
    private void submitShutdownTask() {
        executor.submit(new Runnable() {
                @OverRide
                public void run() {
                    if (!running.compareAndSet(true, false)) {
                        return;
                    }

                    try {
                        ledgerAuditorManager.close();   <- here has already closed LAM.
                    } catch (InterruptedException ie) {
                        Thread.currentThread().interrupt();
                        LOG.warn("InterruptedException while closing ledger auditor manager", ie);
                    } catch (Exception ke) {
                        LOG.error("Exception while closing ledger auditor manager", ke);
                    }
                }
            });
    }
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zymap zymap zymap approved these changes

@hsaputra hsaputra Awaiting requested review from hsaputra

@sijie sijie Awaiting requested review from sijie

@rdhabalia rdhabalia Awaiting requested review from rdhabalia

@eolivelli eolivelli Awaiting requested review from eolivelli

Assignees

@merlimat merlimat

Labels
release/4.14.3 type/improvement
Projects
None yet
Milestone
4.15.0
Development

Successfully merging this pull request may close these issues.
2 participants
@merlimat @zymap