Configure Netty allocator in bookie and client #1755

merlimat · 2018-10-23T19:38:22Z

Motivation

This is based on #1754. Adding the code to configure and use the allocator wrapper in bookie and client.

(I'll rebase once the first PR is merged)

sijie · 2018-10-25T12:07:57Z

@merlimat #1754 is merged. so you can rebase now.

merlimat · 2018-10-25T19:43:58Z

@sijie rebased on master

eolivelli

Looks good to me.
+1 very interesting and useful idea

Please create a task for documentation of this new feature, possibly blocker for 4.9 or just add the documentation in this patch.

merlimat · 2018-10-25T21:03:19Z

run tls tests

merlimat · 2018-10-25T22:27:16Z

run bookkeeper-server tls tests

ivankelly

Looks good. Minor comments.

bookkeeper-server/pom.xml

bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java

ivankelly · 2018-10-26T09:17:31Z

bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java


-    static ByteBuf createExplicitLACEntry(long ledgerId, ByteBuf explicitLac) {
-        ByteBuf bb = PooledByteBufAllocator.DEFAULT.directBuffer(8 + 8 + 4 + explicitLac.capacity());
+    private ByteBuf createExplicitLACEntry(long ledgerId, ByteBuf explicitLac) {


Unrelated to this patch, but we really shouldn't have serialization code in Bookie

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

bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java

bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java

conf/bk_server.conf

…ecksum/CRC32CDigestManager.java Co-Authored-By: merlimat <mmerli@apache.org>

bookkeeper-server/pom.xml

bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java

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

bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java

conf/bk_server.conf

### Motivation * Upgrade to latest Netty version which brings in perf improvements and some new features (eg: netty/netty#8267) * Broke down the dependencies from `netty-all` into individual components, as discussed at #1755 (comment) Reviewers: Ivan Kelly <ivank@apache.org>, Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org> This closes #1784 from merlimat/netty-4.1.31

merlimat · 2018-11-07T00:14:04Z

@sijie @ivankelly Addressed comments, PTAL again.

ivankelly

ship it

bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java

bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java

sijie · 2018-12-03T19:53:57Z

run integration tests
run bookkeeper-server tls tests

sijie · 2018-12-05T20:14:05Z

run bookkeeper-server tls tests

sijie · 2018-12-14T01:51:42Z

@merlimat it seems this PR requires rebase and there are some checkstyle issues needs to be fixed as well.

sijie · 2019-01-06T21:46:01Z

@merlimat it seems this PR requires rebase and there are some checkstyle issues needs to be fixed as well.

merlimat · 2019-01-08T19:49:33Z

run bookkeeper-server bookie tests

coveralls · 2020-12-02T17:43:51Z

Pull Request Test Coverage Report for Build 3657

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-72.08%) to 0.0%

Totals
Change from base Build 323:	-72.08%
Covered Lines:	0
Relevant Lines:	86

💛 - Coveralls

…ding the entry in ReadCache ### Motivation Original PR: #1755, It should be that this PR forgot to modify the memory application method. When the direct memory is insufficient, it does not fall back to the jvm memory, and the bookie hangs directly. ![image](https://user-images.githubusercontent.com/35599757/137859349-f145bb88-7d1c-4739-b6d1-6f8987831cc0.png) ![image](https://user-images.githubusercontent.com/35599757/137859462-4e2b3dc5-3287-4bf7-8dad-048ad8a7723f.png) ### Changes Use `OutOfMemoryPolicy` when the direct memory is insufficient when reading the entry in `ReadCache`. Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Andrey Yegorov <None> This closes #2836 from wenbingshen/useOutOfMemoryPolicyInReadCache

…ding the entry in ReadCache ### Motivation Original PR: apache#1755, It should be that this PR forgot to modify the memory application method. When the direct memory is insufficient, it does not fall back to the jvm memory, and the bookie hangs directly. ![image](https://user-images.githubusercontent.com/35599757/137859349-f145bb88-7d1c-4739-b6d1-6f8987831cc0.png) ![image](https://user-images.githubusercontent.com/35599757/137859462-4e2b3dc5-3287-4bf7-8dad-048ad8a7723f.png) ### Changes Use `OutOfMemoryPolicy` when the direct memory is insufficient when reading the entry in `ReadCache`. Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Andrey Yegorov <None> This closes apache#2836 from wenbingshen/useOutOfMemoryPolicyInReadCache

…ding the entry in ReadCache ### Motivation Original PR: apache#1755, It should be that this PR forgot to modify the memory application method. When the direct memory is insufficient, it does not fall back to the jvm memory, and the bookie hangs directly. ![image](https://user-images.githubusercontent.com/35599757/137859349-f145bb88-7d1c-4739-b6d1-6f8987831cc0.png) ![image](https://user-images.githubusercontent.com/35599757/137859462-4e2b3dc5-3287-4bf7-8dad-048ad8a7723f.png) ### Changes Use `OutOfMemoryPolicy` when the direct memory is insufficient when reading the entry in `ReadCache`. Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Andrey Yegorov <None> This closes apache#2836 from wenbingshen/useOutOfMemoryPolicyInReadCache (cherry picked from commit f19544b)

…ding the entry in ReadCache ### Motivation Original PR: apache#1755, It should be that this PR forgot to modify the memory application method. When the direct memory is insufficient, it does not fall back to the jvm memory, and the bookie hangs directly. ![image](https://user-images.githubusercontent.com/35599757/137859349-f145bb88-7d1c-4739-b6d1-6f8987831cc0.png) ![image](https://user-images.githubusercontent.com/35599757/137859462-4e2b3dc5-3287-4bf7-8dad-048ad8a7723f.png) ### Changes Use `OutOfMemoryPolicy` when the direct memory is insufficient when reading the entry in `ReadCache`. Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Andrey Yegorov <None> This closes apache#2836 from wenbingshen/useOutOfMemoryPolicyInReadCache

merlimat added the type/improvement label Oct 23, 2018

merlimat added this to the 4.9.0 milestone Oct 23, 2018

merlimat self-assigned this Oct 23, 2018

merlimat requested review from dlg99, eolivelli, ivankelly, reddycharan and sijie October 23, 2018 19:38

merlimat force-pushed the use-allocator branch from 0c796ee to 6444893 Compare October 23, 2018 23:54

merlimat added 3 commits October 25, 2018 12:40

Configure Netty allocator in bookie and client

efeb838

Fixed checkstyle issues

6a83596

Included allocator in shading

806b409

merlimat force-pushed the use-allocator branch from 022505a to 806b409 Compare October 25, 2018 19:41

eolivelli approved these changes Oct 25, 2018

View reviewed changes

ivankelly requested changes Oct 26, 2018

View reviewed changes

Update bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/ch…

f1d61da

…ecksum/CRC32CDigestManager.java Co-Authored-By: merlimat <mmerli@apache.org>

sijie requested changes Oct 28, 2018

View reviewed changes

merlimat mentioned this pull request Nov 5, 2018

Upgrade to Netty 4.1.31 and use individual dependencies #1784

Merged

Merge remote-tracking branch 'apache/master' into use-allocator

b9ef5c0

merlimat added 2 commits November 6, 2018 16:03

Addressed comments

0bcb679

Merge remote-tracking branch 'merlimat/use-allocator' into use-allocator

994ef4f

Fixed import order

de6c9a2

ivankelly approved these changes Nov 7, 2018

View reviewed changes

sijie approved these changes Nov 7, 2018

View reviewed changes

bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java Outdated Show resolved Hide resolved

bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java Outdated Show resolved Hide resolved

merlimat force-pushed the use-allocator branch from 0d9551c to 2dbbee3 Compare November 30, 2018 01:25

fixed EntryLogTest

db83ce7

merlimat force-pushed the use-allocator branch from e188c97 to db83ce7 Compare November 30, 2018 01:41

merlimat added 4 commits November 29, 2018 18:41

Fixed checkstyle

6725ce8

Merge remote-tracking branch 'apache/master' into use-allocator

21a57d8

Fixed BookieInitializationTest

1290ecf

Fixed fallback when explicitely asking for direct buffers

e60ba8a

merlimat added 4 commits December 7, 2018 12:29

WIP

b57c28b

Merge remote-tracking branch 'apache/master' into use-allocator

25c5934

Fixed merging issue

e9bd2d3

Removed unused imports

0436d06

merlimat added 3 commits January 7, 2019 15:16

Merge remote-tracking branch 'apache/master' into use-allocator

3e54adf

Fixed checkstyle

6e09642

Merge remote-tracking branch 'apache/master' into use-allocator

21b950c

merlimat added area/bookie area/client release/4.9.0 labels Jan 8, 2019

merlimat merged commit 1a66de0 into apache:master Jan 8, 2019

merlimat deleted the use-allocator branch January 8, 2019 20:16

wenbingshen mentioned this pull request Oct 19, 2021

Use OutOfMemoryPolicy when the direct memory is insufficient when reading the entry in ReadCache #2836

Merged

Configure Netty allocator in bookie and client #1755

Configure Netty allocator in bookie and client #1755

Uh oh!

Conversation

merlimat commented Oct 23, 2018

Motivation

Uh oh!

sijie commented Oct 25, 2018

Uh oh!

merlimat commented Oct 25, 2018

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

merlimat commented Oct 25, 2018

Uh oh!

merlimat commented Oct 25, 2018

Uh oh!

ivankelly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ivankelly Oct 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

merlimat commented Nov 7, 2018

Uh oh!

ivankelly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sijie commented Dec 3, 2018

Uh oh!

sijie commented Dec 5, 2018

Uh oh!

sijie commented Dec 14, 2018

Uh oh!

sijie commented Jan 6, 2019

Uh oh!

merlimat commented Jan 8, 2019

Uh oh!

coveralls commented Dec 2, 2020

Pull Request Test Coverage Report for Build 3657

💛 - Coveralls

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ivankelly Oct 26, 2018 •

edited

Loading