Skip to content

CAP-20: INITENTRY/METAENTRY support for Bucket List#1950

Merged
latobarita merged 20 commits intostellar:masterfrom
graydon:init-entry
Apr 4, 2019
Merged

CAP-20: INITENTRY/METAENTRY support for Bucket List#1950
latobarita merged 20 commits intostellar:masterfrom
graydon:init-entry

Conversation

@graydon
Copy link
Copy Markdown
Contributor

@graydon graydon commented Feb 6, 2019 •
edited
Loading

Description

This adds INITENTRY/METAENTRY support to the bucket list. See the mailing list post for initial discussion, or CAP 20 for current design.

The interesting points here relate to changes to Bucket::merge and its supporting shadowing routine maybePut. I added a fair bit of commentary to help convince myself that the design @MonsieurNicolas proposed is both necessary and sufficient. Would love to see additional eyes look it over / reason through it.

I've done a fair bit of rebasing and squashing and now believe it's about as nice as I can make it. Should be ready for review. The majority of the changes here are testing; the new logic is relatively minimal.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v5.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

Perf summary: it makes the buckets much smaller when you're churning small-lifetime objects. See the [bl-initentry] test results:

Before:

[Bucket INFO] Level 0 size: 81
[Bucket INFO] Level 1 size: 293
[Bucket INFO] Level 2 size: 469
[Bucket INFO] Level 3 size: 1286
[Bucket INFO] Level 4 size: 7168
[Bucket INFO] Level 5 size: 32768
[Bucket INFO] Level 6 size: 38132
[Bucket INFO] Level 7 size: 0
[Bucket INFO] Level 8 size: 0
[Bucket INFO] Level 9 size: 0
[Bucket INFO] Level 10 size: 0

After:

[Bucket INFO] Level 0 size: 79
[Bucket INFO] Level 1 size: 259
[Bucket INFO] Level 2 size: 367
[Bucket INFO] Level 3 size: 394
[Bucket INFO] Level 4 size: 400
[Bucket INFO] Level 5 size: 400
[Bucket INFO] Level 6 size: 300
[Bucket INFO] Level 7 size: 0
[Bucket INFO] Level 8 size: 0
[Bucket INFO] Level 9 size: 0
[Bucket INFO] Level 10 size: 0

@graydon graydon force-pushed the init-entry branch 5 times, most recently from 328132c to bd79f2f Compare February 22, 2019 02:22
jonjove
jonjove reviewed Feb 25, 2019
View reviewed changes
MonsieurNicolas
MonsieurNicolas reviewed Feb 25, 2019
View reviewed changes
MonsieurNicolas
MonsieurNicolas reviewed Feb 25, 2019
View reviewed changes
@graydon graydon force-pushed the init-entry branch 4 times, most recently from f5873c9 to 885ee71 Compare March 6, 2019 00:01
@graydon graydon force-pushed the init-entry branch 4 times, most recently from 1b8ee25 to 03ad0e5 Compare March 14, 2019 00:34
@graydon graydon mentioned this pull request Mar 15, 2019
Merged
@graydon graydon force-pushed the init-entry branch 8 times, most recently from 9a99241 to fdd410f Compare March 22, 2019 08:09
@graydon graydon changed the title [do not merge] bucket: sketch of support for INITENTRY CAP-20: INITENTRY/METAENTRY support for Bucket List Mar 22, 2019
jonjove
jonjove reviewed Mar 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jonjove jonjove left a comment

Choose a reason for hiding this comment

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

The implementation looks good overall and the comment corrections in the last few commits are also welcome. I haven't reviewed the tests yet though.

graydon and others added 14 commits April 3, 2019 17:01
@graydon
bucket: split bucket tests into 3 files (normal, list and manager)
3bea809
@graydon
bucket: add tests of merging with INIT & META in various versions.
b042be5
@graydon
bucket: more thorough stop-start test of buckets.
c9c9c9e
@graydon
history: check initentry-aware version can catch up from older history.
d05493f
@graydon
bucket: default protocol 0, adapt tests to avoid version time-travel.
7bb0db6
@graydon
herder: test 10->11 upgrade.
56b441a
@graydon
bucketlist: correct comment to deal with range skew.
0684f7e
@graydon
bucket: add comments with some more-precise bucket-event boundary num…
cd1ad60 
…bers.
@graydon
clang reformat
e2a5bbb
@graydon
bucket: scale default tests down a touch to not time out CI.
308dd28
@jonjove @graydon
Fixed tests that depend on order of merges in Bucket::fresh
0195502
@jonjove @graydon
Simplify Bucket::fresh
d864df2
@jonjove @graydon
Clean-up usage of convertToBucketEntry
6b5fa51
@graydon
bucket: reformat and fix upgrade test.
02b8111
@graydon graydon force-pushed the init-entry branch 2 times, most recently from 3dd2457 to e0659d7 Compare April 4, 2019 01:33
@graydon
Copy link
Copy Markdown
Contributor Author

graydon commented Apr 4, 2019

r+ e0659d7

@graydon
Copy link
Copy Markdown
Contributor Author

graydon commented Apr 4, 2019

r+ 233106a

@graydon
Copy link
Copy Markdown
Contributor Author

graydon commented Apr 4, 2019

latobarita: retry

@marta-lokhova marta-lokhova mentioned this pull request Apr 4, 2019
Merged
6 tasks
@graydon
Copy link
Copy Markdown
Contributor Author

graydon commented Apr 4, 2019

@latobarita: retry

latobarita added a commit that referenced this pull request Apr 4, 2019
@latobarita
Merge pull request #1950 from graydon/init-entry
dc5fab0 
CAP-20: INITENTRY/METAENTRY support for Bucket List

Reviewed-by: graydon
@latobarita latobarita merged commit 233106a into stellar:master Apr 4, 2019
@graydon graydon deleted the init-entry branch April 11, 2019 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MonsieurNicolas MonsieurNicolas MonsieurNicolas left review comments

+1 more reviewer

@jonjove jonjove jonjove left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@graydon graydon

Labels

protocol

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@graydon @MonsieurNicolas @latobarita @jonjove