DL-2: DistributedLog should work with the official apache bookkeeper by hellostreaming · Pull Request #135 · apache/distributedlog

hellostreaming · 2017-07-27T01:18:05Z

This change is to upgrade bookkeeper version to BK 4.5.0.

upgrade bookkeeper version to 4.5.0-SNAPSHOT (still waiting a few pull requests to merge Issue 296: Bookie supports ephemeral port bookkeeper#297 Introduce BookieClusterManager and Improve BookieListener bookkeeper#287
- change registerSuccessEvent for StatsLogger to add TimeUnit
- use netty4 eventloop
- move twitter repository dependencies to proxy related module only. core library will not depend on scala dependency anymore.

This change is a collaboration change with @sijie

We will provide a performance comparison between 0.4.0 (using Twitter BK) and 0.5.0 (using BK 4.5.) in a separate pull request or email.

- registerSuccessEvent and registerFailureEvent needs TimeUnit - change HashedWheelTimer to netty 4 - change channelFactory to eventLoopGroup

…version

hellostreaming · 2017-08-09T14:16:23Z

local test passed since the bk binrary updating.

sijie · 2017-08-09T17:33:05Z

Awesome @zhaijack !

sijie · 2017-08-16T07:12:53Z

Awesome! all the tests passed!

@leighst can you also take a look at this?

sijie · 2017-08-16T19:02:30Z

pom.xml

    <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
    <!-- dependencies -->
-    <bookkeeper.version>4.3.7-TWTTR-OSS</bookkeeper.version>
+    <bookkeeper.version>4.5.0-SNAPSHOT</bookkeeper.version>


@zhaijack since 4.5.0 is already released, I think we can remove -SNAPSHOT to use the released version.

Thanks. will change it.

leighst · 2017-08-16T19:19:57Z

distributedlog-core/src/main/java/org/apache/distributedlog/impl/BKNamespaceDriver.java

+            try {
+                return new EpollEventLoopGroup(numThreads, threadFactory);
+            } catch (Throwable t) {
+                LOG.warn("Could not use Netty Epoll event loop for bookie server: {}", t.getMessage());


Any reason to not print the stack?

LOG.warn("Could not use Netty Epoll event loop for bookie server: ", t);

Thanks. will change it.

leighst · 2017-08-16T19:23:13Z

distributedlog-core/src/main/java/org/apache/distributedlog/tools/DistributedLogTool.java

                System.out.println("Skip inprogress log segment " + segment);
                return;
            }
-            LedgerHandle lh = bkAdmin.openLedger(segment.getLogSegmentId(), true);


why did we remove the second arg?

Thanks. This is the difference of bookkeeper apache version and twitter version. Apache version does not have the second arg, so need to change all the caller of it in DL.

@leighst @zhaijack - I think this change is related to autorecovery chagnes in twitter branch. I haven't ported those changes. @zhaijack how about we create a github issue for this? and make a comment in the code to track this, once 4.6.0 is out we can fix this.

leighst · 2017-08-16T19:24:34Z

distributedlog-core/src/main/java/org/apache/distributedlog/tools/DistributedLogTool.java

        addCommand(new ReadLastConfirmedCommand());
        addCommand(new ReadEntriesCommand());
-        addCommand(new RecoverCommand());
+        // addCommand(new RecoverCommand());


Would you mind explaining why we need to remove this?
Also i think it would be preferable to link to a fix ticket here.

I think this command depends on some auto reocvery changes that need to be ported back from twitter branch.

@zhaijack can you create a github issue and comment the issue in the code? we can fix it later.

Thanks. #150 is opened to track this issue, will add a comment here in the code.

leighst · 2017-08-16T19:26:26Z

...g-mapreduce/src/main/java/org/apache/distributedlog/mapreduce/DistributedLogInputFormat.java

-/**
- * InputFormat to read data from a distributedlog stream.
- */
-public class DistributedLogInputFormat


Do we have to remove these?

we are trying to provide a real working hdfs wrapper over distributedlog api. so we don't need this module. that would come out in subsequent changes.

leighst · 2017-08-16T19:31:17Z

Do we need to do similar work for ByteBuf in BKLogSegmentEntryWriter?

sijie · 2017-08-17T05:40:03Z

@leighst regarding to fully leverage the bytebuf feature in 4.5.0, we will have subsequent changes to achieve zero memory copy.

sijie and others added 3 commits July 19, 2017 16:33

Bump BK version to 4.5.0-SNAPSHOT

2f25d9f

- registerSuccessEvent and registerFailureEvent needs TimeUnit - change HashedWheelTimer to netty 4 - change channelFactory to eventLoopGroup

DL-2: DistributedLog should work with the official apache bookkeeper …

ccd7448

…version

pass compile

6957e46

hellostreaming changed the title ~~(WIP) DL-2: DistributedLog should work with the official apache bookkeeper~~ DL-2: DistributedLog should work with the official apache bookkeeper Jul 30, 2017

hellostreaming added 2 commits July 30, 2017 23:16

move twttr repo

a047cf6

bump curator to 4.0

67014df

hellostreaming changed the title ~~DL-2: DistributedLog should work with the official apache bookkeeper~~ (WIP)DL-2: DistributedLog should work with the official apache bookkeeper Jul 31, 2017

exclude .repo

892ebe2

hellostreaming changed the title ~~(WIP)DL-2: DistributedLog should work with the official apache bookkeeper~~ DL-2: DistributedLog should work with the official apache bookkeeper Aug 9, 2017

jiazhai added 2 commits August 15, 2017 23:27

Merge remote-tracking branch 'apache/master' into bump_dl_version

d4fdfcf

reduce the number of threads used for tests

df71a83

sijie reviewed Aug 16, 2017

View reviewed changes

leighst reviewed Aug 16, 2017

View reviewed changes

change according comments

cb59cbb

hellostreaming mentioned this pull request Aug 17, 2017

Fix RecoverCommand in tools/DistributedLogTool.java after porting back needed changes #150

Open

add comments to track RecoverCommand

875bc8e

leighst approved these changes Aug 17, 2017

View reviewed changes

sijie added this to the 0.5.0 milestone Aug 17, 2017

sijie added area/core release/0.5.0 type/task labels Aug 17, 2017

sijie assigned hellostreaming Aug 17, 2017

sijie closed this in 09be3fc Aug 17, 2017

sijie mentioned this pull request Sep 4, 2017

failed to compiled in the distributedlog_2.11-0.4.0-incubating #174

Closed

Conversation

hellostreaming commented Jul 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hellostreaming commented Aug 9, 2017

Uh oh!

sijie commented Aug 9, 2017

Uh oh!

sijie commented Aug 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leighst commented Aug 16, 2017

Uh oh!

sijie commented Aug 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hellostreaming commented Jul 27, 2017 •

edited

Loading