Skip to content

Support pass threshold parameters#3778

Open
gaozhangmin wants to merge 2 commits into
apache:masterfrom
gaozhangmin:gc-threshold1
Open

Support pass threshold parameters#3778
gaozhangmin wants to merge 2 commits into
apache:masterfrom
gaozhangmin:gc-threshold1

Conversation

@gaozhangmin

@gaozhangmin gaozhangmin commented Feb 13, 2023

Copy link
Copy Markdown
Contributor

Motivation

/api/v1/bookie/gc TriggerGcService supports majorCompactionThreshold and minorCompactionThreshold majorCompactionMaxTimeMillis , minorCompactionMaxTimeMillis parameters.

Changes

/api/v1/bookie/gc TriggerGcService supports majorCompactionThreshold and minorCompactionThreshold majorCompactionMaxTimeMillis , minorCompactionMaxTimeMillis parameters.

@codecov-commenter

codecov-commenter commented Feb 13, 2023

Copy link
Copy Markdown

Codecov Report

Merging #3778 (27c6971) into master (d6d9212) will increase coverage by 2.03%.
The diff coverage is 70.58%.

@@             Coverage Diff              @@
##             master    #3778      +/-   ##
============================================
+ Coverage     58.23%   60.27%   +2.03%     
- Complexity     5612     5818     +206     
============================================
  Files           467      473       +6     
  Lines         40822    40974     +152     
  Branches       5234     5245      +11     
============================================
+ Hits          23774    24698     +924     
+ Misses        14933    14048     -885     
- Partials       2115     2228     +113     
Flag Coverage Δ
bookie 39.69% <11.76%> (?)
client ?
remaining 29.55% <2.94%> (+0.05%) ⬆️
replication 41.46% <67.64%> (+0.06%) ⬆️
tls 20.94% <2.94%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...va/org/apache/bookkeeper/bookie/LedgerStorage.java 50.00% <ø> (+40.90%) ⬆️
...bookkeeper/bookie/storage/ldb/DbLedgerStorage.java 74.61% <0.00%> (+21.31%) ⬆️
...ie/storage/ldb/SingleDirectoryDbLedgerStorage.java 65.16% <0.00%> (+25.84%) ⬆️
...ache/bookkeeper/bookie/GarbageCollectorThread.java 77.34% <61.53%> (+21.95%) ⬆️
...okkeeper/server/http/service/TriggerGCService.java 78.43% <82.35%> (+15.57%) ⬆️
...he/bookkeeper/bookie/InterleavedLedgerStorage.java 77.81% <100.00%> (+24.06%) ⬆️
.../apache/bookkeeper/bookie/SortedLedgerStorage.java 81.15% <100.00%> (+7.24%) ⬆️
...a/org/apache/bookkeeper/client/api/BookKeeper.java 0.00% <0.00%> (-100.00%) ⬇️
.../bookkeeper/proto/checksum/DummyDigestManager.java 0.00% <0.00%> (-100.00%) ⬇️
...okkeeper/client/BookieAddressResolverDisabled.java 0.00% <0.00%> (-100.00%) ⬇️
... and 219 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zymap

zymap commented Feb 14, 2023

Copy link
Copy Markdown
Member

Overall looks good to me. Could you please add a test which the request threshold is different from the configured threshold

@dlg99 dlg99 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.

@hangc0276 hangc0276 requested a review from zymap February 20, 2023 10:42
@hangc0276

Copy link
Copy Markdown
Contributor

@horizonzy Please help take a look, thanks.

@gaozhangmin gaozhangmin requested review from hangc0276 and removed request for eolivelli and zymap February 21, 2023 02:54
@zymap zymap added this to the 4.16.0 milestone Feb 22, 2023

@StevenLuMT StevenLuMT 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.

LGTM

@hangc0276 hangc0276 modified the milestones: 4.16.0, 4.17.0 Mar 13, 2023

@horizonzy horizonzy 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.

LGTM

@gaozhangmin gaozhangmin requested a review from hangc0276 March 16, 2023 03:27
@gaozhangmin gaozhangmin force-pushed the gc-threshold1 branch 2 times, most recently from 70c6f77 to 7103abb Compare March 22, 2023 09:56
@gaozhangmin

Copy link
Copy Markdown
Contributor Author

rerun failure checks

@gaozhangmin

Copy link
Copy Markdown
Contributor Author

@hangc0276 PTAL

@gaozhangmin

Copy link
Copy Markdown
Contributor Author

rerun failure checks

@hezhangjian

Copy link
Copy Markdown
Member

@eolivelli @hangc0276 @merlimat @nicoloboschi @zhaijack Please also take a look

@eolivelli eolivelli removed this from the 4.17.0 milestone May 3, 2024
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.

9 participants