Skip to content

BP-42: New Client API - list ledgers#2457

Merged
eolivelli merged 10 commits into
apache:masterfrom
nicoloboschi:fix/2422/bp-42-list-ledgers
Oct 29, 2020
Merged

BP-42: New Client API - list ledgers#2457
eolivelli merged 10 commits into
apache:masterfrom
nicoloboschi:fix/2422/bp-42-list-ledgers

Conversation

@nicoloboschi

Copy link
Copy Markdown
Contributor

Implementation of BP-42

Motivation

Enhance new client API, added methods for

  • listing ledgers
  • access to ledger metadata

Changes

  • Added new interfaces in "api" client package for ledgers listing
  • Added new method getLedgerMetadata(long ledgerId) to BookKeeper interface
  • Added getLedgerId() to LedgerMetadata type
    -- Ledger metadata ser/deser are not changed since ledgerId is not stored inside ZK data node

Updated all tests using LedgerMetadataBuilder because ledgerId is now required in order to call build() method

Master Issue: #2422

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work.
I left a few comments, mostly nits

@jiazhai @sijie @Ghatage PTAL

Comment thread bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java Outdated

@jiazhai jiazhai left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@nicoloboschi

Copy link
Copy Markdown
Contributor Author

@eolivelli I made some changes as requested

Checks passes except for SlowBookieTest but I think it is flaky, running it locally passes

@eolivelli

Copy link
Copy Markdown
Contributor

test failures seem related to this patch

Error:  org.apache.bookkeeper.meta.AbstractZkLedgerManagerTest.testReadLedgerMetadataSuccess
Error:    Run 1: AbstractZkLedgerManagerTest.testReadLedgerMetadataSuccess:361 expected: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}> but was: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}>
Error:    Run 2: AbstractZkLedgerManagerTest.testReadLedgerMetadataSuccess:361 expected: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}> but was: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}>
Error:    Run 3: AbstractZkLedgerManagerTest.testReadLedgerMetadataSuccess:361 expected: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}> but was: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}>
[INFO] 
Error:  org.apache.bookkeeper.meta.AbstractZkLedgerManagerTest.testUnregisterLedgerMetadataListener
Error:    Run 1: AbstractZkLedgerManagerTest.testUnregisterLedgerMetadataListener:809 expected: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}> but was: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}>
Error:    Run 2: AbstractZkLedgerManagerTest.testUnregisterLedgerMetadataListener:809 expected: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}> but was: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}>
Error:    Run 3: AbstractZkLedgerManagerTest.testUnregisterLedgerMetadataListener:809 expected: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}> but was: org.apache.bookkeeper.client.LedgerMetadataImpl<LedgerMetadata{formatVersion=3, ensembleSize=5, writeQuorumSize=3, ackQuorumSize=3, state=OPEN, digestType=CRC32C, password=base64:, ensembles={0=[192.0.2.1:3181, 192.0.2.2:3181, 192.0.2.3:3181, 192.0.2.4:3181, 192.0.2.5:3181]}, customMetadata={}}>
[INFO] 
Error:  Errors: 
Error:  org.apache.bookkeeper.meta.TestLedgerMetadataSerDe.testPeggedToV3SerDe
Error:    Run 1: TestLedgerMetadataSerDe.testPeggedToV3SerDe:133 » IllegalArgument Ledger id mu...
Error:    Run 2: TestLedgerMetadataSerDe.testPeggedToV3SerDe:133 » IllegalArgument Ledger id mu...
Error:    Run 3: TestLedgerMetadataSerDe.testPeggedToV3SerDe:133 » IllegalArgument Ledger id mu...
[INFO] 
Error:  org.apache.bookkeeper.meta.TestLedgerMetadataSerDe.testStoreSystemtimeAsLedgerCtimeDisabledWithNewerVersion
Error:    Run 1: TestLedgerMetadataSerDe.testStoreSystemtimeAsLedgerCtimeDisabledWithNewerVersion:171 » IllegalArgument
Error:    Run 2: TestLedgerMetadataSerDe.testStoreSystemtimeAsLedgerCtimeDisabledWithNewerVersion:171 » IllegalArgument
Error:    Run 3: TestLedgerMetadataSerDe.testStoreSystemtimeAsLedgerCtimeDisabledWithNewerVersion:171 » IllegalArgument
[INFO] 
Error:  org.apache.bookkeeper.meta.TestLedgerMetadataSerDe.testStoreSystemtimeAsLedgerCtimeEnabledWithNewerVersion
Error:    Run 1: TestLedgerMetadataSerDe.testStoreSystemtimeAsLedgerCtimeEnabledWithNewerVersion:151 » IllegalArgument
Error:    Run 2: TestLedgerMetadataSerDe.testStoreSystemtimeAsLedgerCtimeEnabledWithNewerVersion:151 » IllegalArgument
Error:    Run 3: TestLedgerMetadataSerDe.testStoreSystemtimeAsLedgerCtimeEnabledWithNewerVersion:151 » IllegalArgument

@eolivelli

Copy link
Copy Markdown
Contributor

rerun-failure-checks

@eolivelli

Copy link
Copy Markdown
Contributor

rerun failure checks

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants