bookkeeper.git
2 hours ago[CI] Cleanup workspace before build master
Sijie Guo [Mon, 23 Jul 2018 05:53:38 +0000 (13:53 +0800)] 
[CI] Cleanup workspace before build

Descriptions of the changes in this PR:

*Motivation*

A lot of CI jobs failed due to gitlock file left in the workspace

*Changes*

Cleanup workspace before build

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1566 from sijie/cleanup_workspace

2 days agoDisable testConcurrentWriteAndReadCalls in EntryLogTest
Sijie Guo [Fri, 20 Jul 2018 11:51:48 +0000 (13:51 +0200)] 
Disable testConcurrentWriteAndReadCalls in EntryLogTest

Descriptions of the changes in this PR:

*Motivation*

Those tests passed locally. However they are taking very long time to complete and cause all CI jobs become very flaky.
In order to unblock all PRs, disable these tests in EntryLogTest.

*Changes*

Marked 4 tests in EntryLogTest as `FlakyTest`.

Relate Issue: #1516

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1561 from sijie/disable_some_entrylog_test

5 days ago[TABLE SERVICE] Fix Journal replay issue on range deletion
Sijie Guo [Wed, 18 Jul 2018 07:15:05 +0000 (00:15 -0700)] 
[TABLE SERVICE] Fix Journal replay issue on range deletion

Descriptions of the changes in this PR:

*Motivation*

We use `\000` as an indicator for null key in the protobuf delete operation.
However when we convert the protobuf delete request to an delete operation,
we didn't take `\000` into account.

*Changes*

- Fix ProtoDeleteOpImpl and ProtoRangeOpImpl to handle '\000'

(have a separated PR for an integration test to ensure this work)

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1554 from sijie/fix_journal_replay

5 days ago[TABLE SERVICE] Support getStreamById in root range store.
Sijie Guo [Wed, 18 Jul 2018 07:14:19 +0000 (00:14 -0700)] 
[TABLE SERVICE] Support getStreamById in root range store.

Descriptions of the changes in this PR:

*Motivation*

Stream readers need to fetch metadata information by quering its metadata using stream id.

*Changes*

Add support in root range to get stream properties using stream id.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1550 from sijie/add_get_stream_by_id

6 days agoISSUE #1534: LedgerCache should be flushed
cguttapalem [Tue, 17 Jul 2018 07:36:30 +0000 (00:36 -0700)] 
ISSUE #1534: LedgerCache should be flushed

Descriptions of the changes in this PR:

### Motivation

EntryLogComparator.CompactionScannerFactory.flush just calls "ledgerStorage.updateEntriesLocations(offsets);" but not "ledgerStorage.flushEntriesLocationsIndex()".

Because of this, EntryLogCompactor.compact method would remove compacted entryLog without updated offsets/locations getting flushed/persisted/fsynced to LedgerCache (Index/FileInfo files). This could lead to data corruption/loss if Bookie is broughtdown/killed before those updated offsets/locations are flushed/persisted/fsynced.
### Changes

In EntryLogComparator, before removing compacted entryLog, LedgerCache (IndexInMemPageMgr and Index files) should be flushed, just like EntryLogger.

Master Issue: #1534

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Matteo Merli <mmerli@apache.org>, Sijie Guo <sijie@apache.org>

This closes #1536 from reddycharan/compactfix, closes #1534

6 days agoISSUE #1537: fix distributedlog incubator link
Jia Zhai [Tue, 17 Jul 2018 07:34:27 +0000 (00:34 -0700)] 
ISSUE #1537: fix distributedlog incubator link

Descriptions of the changes in this PR:
### Motivation

In bookkeeper website, we are still using incubator locations for dlog references. we should change it to http://bookkeeper.apache.org/distributedlog

### Changes
change links from
`https://distributedlog.incubator.apache.org`
to
`https://bookkeeper.apache.org/distributedlog`

Master Issue: #1537

Author: Jia Zhai <zhaijia@apache.org>

Reviewers: Sijie Guo <sijie@apache.org>

This closes #1545 from jiazhai/issue_1537, closes #1537

6 days agoISSUE #1542: Unable to run 'bookkeeper' shell script from /bookkeeper/bin folder
Sijie Guo [Tue, 17 Jul 2018 07:32:27 +0000 (00:32 -0700)] 
ISSUE #1542: Unable to run 'bookkeeper' shell script from /bookkeeper/bin folder

Descriptions of the changes in this PR:

*Bug*

```
Getting NoClassDefFoundError while trying to run 'bookkeeper' shell script from 'bin' folder. Issue seems to be related with classpath (cached classpath file - cached_classpath.txt)

~/Workspace/Community/bookkeeper/bin$ ./bookkeeper shell
JAVA_HOME not set, using java from PATH. (/usr/bin/java)
cat: bookkeeper-server/target/cached_classpath.txt: No such file or directory
Error: A JNI error has occurred, please check your installation and try again
Exception in thread "main" java.lang.NoClassDefFoundError: org/apache/commons/configuration/Configuration
at java.lang.Class.getDeclaredMethods0(Native Method)
at java.lang.Class.privateGetDeclaredMethods(Class.java:2701)
at java.lang.Class.privateGetMethodRecursive(Class.java:3048)
at java.lang.Class.getMethod0(Class.java:3018)
at java.lang.Class.getMethod(Class.java:1784)
at sun.launcher.LauncherHelper.validateMainClass(LauncherHelper.java:544)
at sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:526)
Caused by: java.lang.ClassNotFoundException: org.apache.commons.configuration.Configuration
at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:349)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
... 7 more
```

*Motivation*

Fixes #1542

*Problem*

`bin/common.sh` uses a relative path for caching classpath and locating the pom file.

*Changes*

- change to use full path for cached classpath file and the module pom file
- add a test case to cover this problem

Master Issue: #1542

Author: Sijie Guo <sijie@apache.org>

Reviewers: Charan Reddy Guttapalem <reddycharan18@gmail.com>

This closes #1548 from sijie/fix_issue_1542, closes #1542

12 days agoWebsite - Add other usecases to the overview page
Enrico Olivelli [Wed, 11 Jul 2018 05:32:14 +0000 (22:32 -0700)] 
Website - Add other usecases to the overview page

Add Twitter Manhattan and Herddb.org Distributed Databases usecases for BookKeeper as Write-Ahead-Log

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: Sijie Guo <sijie@apache.org>

This closes #1539 from eolivelli/fix/websiteusacase

3 weeks ago[RELEASE] Bump bk version to 4.7.1 in docker file
Sijie Guo [Tue, 26 Jun 2018 21:23:43 +0000 (14:23 -0700)] 
[RELEASE] Bump bk version to 4.7.1 in docker file

Descriptions of the changes in this PR:

Bump bk version to 4.7.1 to build docker image for `4.7.1`

Author: Sijie Guo <sijieapache.org>

Reviewers: Jia Zhai <None>

This closes #1523 from sijie/bump_docker_version

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1529 from sijie/cherry-pick_docker_change

4 weeks agoLedgerManagerIteratorTest: avoid ledger collision
Samuel Just [Wed, 20 Jun 2018 15:36:51 +0000 (08:36 -0700)] 
LedgerManagerIteratorTest: avoid ledger collision

For the non-LHLM managers, the ledger space seems to be small enough to
allow collisions to occasionally cause a test failure.  Remember
created ledgers to avoid collisions.

(bug W-5104859)
Signed-off-by: Samuel Just <sjustsalesforce.com>
Author: Samuel Just <sjust@salesforce.com>

Reviewers: Charan Reddy Guttapalem <reddycharan18@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1526 from athanatos/forupstream/wip-ledger-manager-iterator-test

4 weeks ago[WEBSITE] update release guide to reflect current release process
Sijie Guo [Wed, 20 Jun 2018 01:13:09 +0000 (18:13 -0700)] 
[WEBSITE] update release guide to reflect current release process

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1525 from sijie/fix_release_guide

4 weeks agoISSUE #1495: Option to enforce minNumRacksPerWriteQuorum
cguttapalem [Wed, 20 Jun 2018 01:08:26 +0000 (18:08 -0700)] 
ISSUE #1495: Option to enforce minNumRacksPerWriteQuorum

Descriptions of the changes in this PR:

Provide an option to enforce the guarantee of minNumRacksPerWriteQuorum
if it is enabled. If it cann't find a bookie to enforce the guarantee
then the API in RackawareEnsemblePlacementPolicy should throw
BKNotEnoughBookiesException.

Master Issue: #1495

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Sijie Guo <sijie@apache.org>

This closes #1496 from reddycharan/enforceminracks, closes #1495

4 weeks ago[RELEASE] update website with 4.7.1 documentation
Sijie Guo [Tue, 19 Jun 2018 21:10:14 +0000 (14:10 -0700)] 
[RELEASE] update website with 4.7.1 documentation

Descriptions of the changes in this PR:

Signed-off-by: Sijie Guo <sijieapache.org>
Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1522 from sijie/update_website

4 weeks agoISSUE #705: add site/scripts/release_minor.sh for minor releases
Sijie Guo [Tue, 19 Jun 2018 18:15:53 +0000 (11:15 -0700)] 
ISSUE #705: add site/scripts/release_minor.sh for minor releases

Descriptions of the changes in this PR:

*Motivation*

Currently `site/scripts/release.sh` only supports major releases.

*Changes*

Added a similar `release_minor.sh` to support minor releases.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1524 from sijie/provide_script_release_minor, closes #705

4 weeks ago[RELEASE] Release Notes for Release 4.7.1
Sijie Guo [Tue, 19 Jun 2018 18:13:37 +0000 (11:13 -0700)] 
[RELEASE] Release Notes for Release 4.7.1

Descriptions of the changes in this PR:

Release Notes for release 4.7.1

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1518 from sijie/release_notes_471

4 weeks agoFix typo in bk_server.conf
Ivan Kelly [Mon, 18 Jun 2018 18:16:15 +0000 (11:16 -0700)] 
Fix typo in bk_server.conf

Author: Ivan Kelly <ivank@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1520 from ivankelly/ivankelly-patch-1

5 weeks agoBP-14 force() API - client side implementation
Enrico Olivelli [Mon, 18 Jun 2018 07:05:11 +0000 (09:05 +0200)] 
BP-14 force() API - client side implementation

- Introduce the client side force() API
- Implementation on the client side wire protocol for FORCE_LEDGER RPC
- Disable ensemble changes for DEFERRED_SYNC writers
- Prevent v2 client from using force() API.

The force() API enables the client (usually with DEFERRED_SYNC write flags) to require a point of synchronization with all the bookies in the ensemble, to have guarantees about durability of previously written entries (and ackknowledgerd), this way LastAddConfirmed is able to advance.

For DEFERRED_SYNC writers LastAddConfirmed will advance only using this API

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: Jia Zhai <None>, Sijie Guo <sijie@apache.org>, Venkateswararao Jujjuri (JV) <None>

This closes #1436 from eolivelli/bp14-force-client-api

5 weeks agoMove TimedOutTestsListener from dlog to bookkeeper-common
Sijie Guo [Thu, 14 Jun 2018 14:57:33 +0000 (16:57 +0200)] 
Move TimedOutTestsListener from dlog to bookkeeper-common

Descriptions of the changes in this PR:

This PR is to debug CI problems observed at #1516

### Motivation

dlog uses a `TimedOutTestsListener` to dump the jvm stack when a junit test timed out.
move this class to bookkeeper-common and built with `test-jar`, so it can be used across the project.

### Changes

relocate `TimedOutTestsListener` and its related classes from distributedlog-common to bookkeeper-common.

Related Issue: #1516

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1517 from sijie/add_test_timedout_listener

5 weeks agoISSUE #1476: LedgerEntry is recycled twice at ReadLastConfirmedAndEntryOp
Sijie Guo [Thu, 14 Jun 2018 09:13:22 +0000 (02:13 -0700)] 
ISSUE #1476: LedgerEntry is recycled twice at ReadLastConfirmedAndEntryOp

Descriptions of the changes in this PR:

The issue #1476 is caused by peculative reads with object recycling, same request object will be added to the CompletionObjects multiple times with different txnid.  In fact the logic of process the request already take this into account, only on place inside `ReadLastConfirmedAndEntryOp.requestComplete` forget to check requestComplete before calling `submitCallback` which in turn call request.close.

### Motivation

to fix #1476

### Changes

check `requestComplete` before `submitCallback` in `ReadLastConfirmedAndEntryOp.requestComplete`

Master Issue: #1476

Author: Sijie Guo <sijie@apache.org>
Author: infodog <infodog@hotmail.com>
Author: zhengxiangyang <zxy@xinshi.net>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1509 from infodog/issue1476, closes #1476

5 weeks agoIssue #1489: Better Prevent Read Outliers during short-term Bookie Slow-Down
Nicolas Michael [Thu, 14 Jun 2018 00:41:02 +0000 (17:41 -0700)] 
Issue #1489: Better Prevent Read Outliers during short-term Bookie Slow-Down

### Motivation

Bookies can temporarily be slow for a large number of reasons, often for just a brief time of few milliseconds to seconds such as during Java Garbage Collection or EntryLog compaction. For writes, latencies of individual bookies are masked by acknowledging the client after a quorum of bookies have replied. However for reads, we don't have any equivalent feature to mask short-term latencies of individual bookies yet (in case of SequenceReadRequests). This PR implements such a feature by reordering reads to prefer bookies with a high probability of being fast over bookies that are potentially slow.

### Changes
This change implements a configurable reordering of read requests in Bokkeeper client based on the number of pending requests to each bookie that could service the request. The intention is to mask the latency of one bookie by directing a read request to another bookie that could potentially service the request faster. This should help prevent read time outliers due to bookies that temporarily are responsing slow, for example due to Java garbage collection, compaction, or any other kind of hickup. Unlike the implementation for Issue #709, this algorithm quickly reacts to both an or decrease increase in queue length of a bookie relative to others, and allows to redirect requests long before they would hit the speculativeReadTimeout. Once the problem is resolved (e.g. Java GC finished), it will quickly direct requests to the previously "slow" bookie as its queue length decreases.

Reordering of reads is based on a threshold of relative queue length to other bookies. Setting the threshold very low will more frequently reorder the read set and potentially result in better latency, but will also reduce data affinity of reads. Reads send to other than the preferred bookie have a low chance to be served from file system cache on that bookie, and will likely result in a physical read. Small thresholds therefore shuffle read requests more among bookies and may lead to reduced file system cache reach and increased physical reads on disks. A larger timeout will maintain data affinity and avoid above problems, but only kick in once a bookie has built-up a considerable queue of requests. It therefore masks only slightly larger outliers, and leads to overall better efficiency.

Master Issue: #1489

Author: Nicolas Michael <nmichael@salesforce.com>

Reviewers: Yiming Zang <yzang2016@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1504 from nicmichael/ReadReordering, closes #1489

5 weeks agoAllow to configure read-only mode in ZooKeeperClient
Sijie Guo [Thu, 14 Jun 2018 00:35:34 +0000 (17:35 -0700)] 
Allow to configure read-only mode in ZooKeeperClient

Underlying `Zookeeper` instance has an option to specify the client is fine to remain connected when the ZK quorum is lost, just in read-only mode.

We should expose the "allow read-only mode" option in `ZooKeeperClient`.

One example of use case for this flag is when connecting to a ZK ensemble that is just used for configuration/metadata store, in which no ephemeral nodes are used. It's therefore better to keep the connection with ZK and be able to keep reading (possibly stale) data from ZK rather than no read at all. Concrete use case is for ZK session for Pulsar global ZK ensemble, where we read configuration data and we don't really need to have a valid session when brokers are trying to read.

Author: Sijie Guo <sijie@apache.org>
Author: Matteo Merli <mmerli@apache.org>

Reviewers: Ivan Kelly <ivank@apache.org>, Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1344 from merlimat/zk-client-read-only

5 weeks agoIssue #1511: Upgrade nokogiri to version 1.8.2
Ivan Kelly [Wed, 13 Jun 2018 08:42:46 +0000 (10:42 +0200)] 
Issue #1511: Upgrade nokogiri to version 1.8.2

Version <1.8.2 contain a security vulnerability.

Author: Ivan Kelly <ivank@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1515 from ivankelly/nokogiri, closes #1511

5 weeks agoISSUE #1490: BookieJournalForceTest is flaky
Sijie Guo [Wed, 13 Jun 2018 02:29:15 +0000 (19:29 -0700)] 
ISSUE #1490: BookieJournalForceTest is flaky

Descriptions of the changes in this PR:

### Motivation

The test cases in BookieJournalForceTest check if the forceWriteQueue is empty after everything is flushed.
However if `adaptiveGroupWrites` is enabled, the forceWrite thread will insert a marker entry into force write queue,
this will fail the forceWriteQueue length checking.

### Changes

Disable `adaptiveGroupWrites` in BookieJournalForceTest

Master Issue: #1490

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1512 from sijie/fix_journalforcetest, closes #1490

5 weeks ago[TABLE SERVICE] replaying TxnRequest is not implemented
Sijie Guo [Tue, 12 Jun 2018 04:48:02 +0000 (21:48 -0700)] 
[TABLE SERVICE] replaying TxnRequest is not implemented

*Motivation*

when enabling table service for pulsar at apache/incubator-pulsar#1922, I noticed that replaying TxnRequest is missed somehow.

*Solution*

This PR implements the replaying TxnRequest logic in the command process.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1505 from sijie/replay_txn_request

5 weeks ago[TABLE SERVICE] Fix StringUtf8Coder and add VarIntCoder
Sijie Guo [Mon, 11 Jun 2018 17:41:31 +0000 (10:41 -0700)] 
[TABLE SERVICE] Fix StringUtf8Coder and add VarIntCoder

Descriptions of the changes in this PR:

*Motivation*

The getSerializeLen() returned by StringUtf8Coder is different from the serialized buffer size.

*Changes*

- Fix StringUtf8Coder
- Add tests to coders
- Add a VarIntCoder

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1507 from sijie/fix_string_coder

5 weeks agoIssue #1500: PendingReadOp.logErrorAndReattemptRead logs errors with debug level...
Andrey Yegorov [Mon, 11 Jun 2018 17:08:03 +0000 (10:08 -0700)] 
Issue #1500: PendingReadOp.logErrorAndReattemptRead logs errors with debug level, some of them useful for troubleshooting at info/warn level

(bug W-3976846)

Descriptions of the changes in this PR:

Changed log level of one line from debug to info

### Motivation

Useful for troubleshooting of prod issue, don't want to have debug log enabled in prod all the time.

### Changes

Changed log level of one line from debug to info

Master Issue: #1500

Author: Andrey Yegorov <ayegorov@salesforce.com>

Reviewers: Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1501 from dlg99/fix/pendingreadop-log, closes #1500

6 weeks ago[TABLE SERVICE] Fix ZkClusterMetadataStoreTest
Sijie Guo [Sun, 10 Jun 2018 19:13:50 +0000 (12:13 -0700)] 
[TABLE SERVICE] Fix ZkClusterMetadataStoreTest

Descriptions of the changes in this PR:

### Motivation

initialize signature was changed. but the test was not changed.

### Changes

Fix the test case.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1506 from sijie/fix_zk_cluster_initialize

6 weeks agoIssue #1498: Log messages/exception text in DiskChecker shows 'used <threshold' in...
Andrey Yegorov [Fri, 8 Jun 2018 02:20:11 +0000 (19:20 -0700)] 
Issue #1498: Log messages/exception text in DiskChecker shows 'used <threshold' in 'used > threshold' situations

(bug W-3812505)

Descriptions of the changes in this PR:

Fixed log/exception text.

### Motivation

Reduced confusion when debugging an issue/going through the logs.

### Changes

replaced '<' with '>'

Master Issue: #1498

Author: Andrey Yegorov <ayegorov@salesforce.com>

Reviewers: Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1499 from dlg99/fix/diskchecker-log, closes #1498

6 weeks ago[DEVTOOLS] Use ASF Jenkins API properly
Sijie Guo [Fri, 8 Jun 2018 01:45:30 +0000 (18:45 -0700)] 
[DEVTOOLS] Use ASF Jenkins API properly

Descriptions of the changes in this PR:

According to https://cwiki.apache.org/confluence/display/INFRA/Using+the+ASF+Jenkins+API,
we need to include 'tree' or 'depth' when using ASF jenkins API.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1503 from sijie/access_jenkins_apis_should_include_depth

6 weeks ago[WEBSITE] Include trademark attribution in footer
Sijie Guo [Fri, 8 Jun 2018 01:43:07 +0000 (18:43 -0700)] 
[WEBSITE] Include trademark attribution in footer

Descriptions of the changes in this PR:

According to http://www.apache.org/foundation/marks/pmcs#attributions, include trademark attribution in the footer.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>, Matteo Merli <mmerli@apache.org>

This closes #1502 from sijie/update_trademarks

6 weeks ago[BOOKIE] [DBLEDGERSTORAGE] fix illegal reference count exception on filling read...
Sijie Guo [Thu, 7 Jun 2018 06:20:38 +0000 (23:20 -0700)] 
[BOOKIE] [DBLEDGERSTORAGE] fix illegal reference count exception on filling read cache

Descriptions of the changes in this PR:

*Problem*

ByteBuf is released at a finally block. DbLedgerStorage doesn't have to release it on return. If it does so, that will cause
double-release and it will throw IllegalReferenceCountException.

*Solution*

Remove `ByteBuf.release` at return.

Related Issue: #1487

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>, Matteo Merli <mmerli@apache.org>

This closes #1488 from sijie/fix_double_release_on_db_ledgerstorage

6 weeks ago[CI] Allow selecting a list of precommit checks to skip in the pull request description.
Sijie Guo [Thu, 7 Jun 2018 06:17:00 +0000 (23:17 -0700)] 
[CI] Allow selecting a list of precommit checks to skip in the pull request description.

Descriptions of the changes in this PR:

*Motivation*

precommit checks are good. however sometime it takes a long time to wait for precommit checks to complete,
for trivial changes (e.g. documentation, ci jenkins files, scripts, proposals).

*Changes*

This PR updates the PULL_REQUEST_TEMPLATE.md to provide a list of precommit checks to be allowed to skip.
So people can choose skip some precommit checks when filing a pull request.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1494 from sijie/skip_tests_is_too_general

6 weeks ago[TABLE SERVICE] Move table service cli to bookkeeper-tools
Sijie Guo [Thu, 7 Jun 2018 06:10:18 +0000 (23:10 -0700)] 
[TABLE SERVICE] Move table service cli to bookkeeper-tools

Descriptions of the changes in this PR:

*Motivation*

This PR follows #1478  to move table service cli to bookkeeper-tools.
So different service components are now available in one cli tool.

*Changes*

- move stream/cli module to tools/stream module
- organize the table service commands as groups:

* cluster
* namespace
* table
* tables

*Output*

Example output:

```
$ bin/bkctl
bkctl interacts and operates Apache BookKeeper clusters

Usage:  bkctl [flags] [command group] [commands]

Commands:

    bookie          Commands on operating a single bookie
    bookies         Commands on operating a cluster of bookies
    cluster         Commands on administrating bookkeeper clusters
    ledger          Commands on interacting with ledgers
    namespace       Commands on operating namespaces
    table           Commands on interacting with tables
    tables          Commands on operating tables

    help            Display help information about it

Flags:

    -c, --conf
        Configuration file

    -n, --namespace
        Namespace scope to run commands (only valid for table service for now)

    -u, --service-uri
        Service Uri

    -h, --help
        Display help information

Use "bkctl [command] --help" or "bkctl help [command]" for more information
about a command
```

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1485 from sijie/stream_cli

6 weeks agoISSUE #1491: fix GLIBC_2.14 dep issue for protoc-3.5.1-linux-x86_64
Ethan Li [Thu, 7 Jun 2018 06:06:23 +0000 (23:06 -0700)] 
ISSUE #1491: fix GLIBC_2.14 dep issue for protoc-3.5.1-linux-x86_64

Descriptions of the changes in this PR:

I tried to build the bookkeeper branch-4.7 on RHEL6 and ran into:

```
 /lib64/libc.so.6: version `GLIBC_2.14' not found (required by bookkeeper-proto/target/protoc-plugins/protoc-3.5.1-linux-x86_64.exe)
```
This is similar to google/protobuf#4109 and it's related to google/protobuf#4138

We can take advantage of pulsar's fix apache/incubator-pulsar#1914 here

Master Issue:  https://github.com/apache/bookkeeper/issues/1491

Author: Ethan Li <ethanopensource@gmail.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1492 from Ethanlm/ISSUE-1491, closes #1491

6 weeks agoRegionAwareEnsemblePlacementPolicy class should use super class's minNumRacksPerWrite...
cguttapalem [Thu, 7 Jun 2018 06:05:07 +0000 (23:05 -0700)] 
RegionAwareEnsemblePlacementPolicy class should use super class's minNumRacksPerWriteQuorum variable

Descriptions of the changes in this PR:

RegionAwareEnsemblePlacementPolicy class should use minNumRacksPerWriteQuorum
variable which is declared in its super class - RackawareEnsemblePlacementPolicyImpl,
since it is related to its super class.

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Sijie Guo <sijie@apache.org>

This closes #1484 from reddycharan/fixregpp

6 weeks ago[TABLE SERVICE] dlog configuration settings are not propagated
Sijie Guo [Thu, 7 Jun 2018 06:03:48 +0000 (23:03 -0700)] 
[TABLE SERVICE] dlog configuration settings are not propagated

Descriptions of the changes in this PR:

*Motivation*

When stream storage server starts the bookie watcher service, it waits for at least `ensembleSize` bookies
available before starting other components. However the dlog settings are not propagated correctly. so when
using this component in an environment that only has 1 bookie (e.g. pulsar standalone). Startup will be hanging
on waiting 3 bookies.

*Solution*

load the dlog settings from the base configuration.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1486 from sijie/fix_loading_dlog_configuration

6 weeks ago[TABLE SERVICE] Improve logging message on initializing table range stores.
Sijie Guo [Thu, 7 Jun 2018 06:02:52 +0000 (23:02 -0700)] 
[TABLE SERVICE] Improve logging message on initializing table range stores.

Descriptions of the changes in this PR:

*Motivation*

Exceptions might be throwing on starting/stopping table ranges. It would be good to have logging messages about the state transition for debugging purpose.

*Build*

Only touch following stream modules, skip bookkeeper-server related tests:

[skip tests]

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1493 from sijie/improve_logging

6 weeks agoImprove merge script on checking the statuses of github checks (missing commit)
Sijie Guo [Thu, 7 Jun 2018 05:40:33 +0000 (22:40 -0700)] 
Improve merge script on checking the statuses of github checks (missing commit)

    Descriptions of the changes in this PR:

    *Motivation*

    Sometimes Jenkins CI job might already succeed, however they failed on publishing the build status
    to Github due to timeout (one example is attached below). In this case, we don't need to rerun the jenkins check again.

    ```
    Setting status of 55c1d54f3293e5166c59f238afd3fb3d98c7eb67 to SUCCESS with url https://builds.apache.org/job/bookkeeper_precommit_client_tests/21/ and message: 'SUCCESS
     '
    Using context: Jenkins: Client Tests
    Could not update commit status of the Pull Request on GitHub.
    org.kohsuke.github.HttpException: Server returned HTTP response code: -1, message: 'null' for URL: https://api.github.com/repos/apache/bookkeeper/statuses/55c1d54f3293e5166c59f238afd3fb3d98c7eb67
            at org.kohsuke.github.Requester.parse(Requester.java:633)
            at org.kohsuke.github.Requester.parse(Requester.java:631)
            at org.kohsuke.github.Requester.parse(Requester.java:631)
            at org.kohsuke.github.Requester.parse(Requester.java:594)
            at org.kohsuke.github.Requester._to(Requester.java:272)
            at org.kohsuke.github.Requester.to(Requester.java:234)
            at org.kohsuke.github.GHRepository.createCommitStatus(GHRepository.java:1075)
            at org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus.createCommitStatus(GhprbSimpleStatus.java:281)
            at org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus.onBuildComplete(GhprbSimpleStatus.java:239)
            at org.jenkinsci.plugins.ghprb.GhprbBuilds.onCompleted(GhprbBuilds.java:205)
            at org.jenkinsci.plugins.ghprb.GhprbBuildListener.onCompleted(GhprbBuildListener.java:28)
            at hudson.model.listeners.RunListener.fireCompleted(RunListener.java:211)
            at hudson.model.Run.execute(Run.java:1769)
            at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
            at hudson.model.ResourceController.execute(ResourceController.java:97)
            at hudson.model.Executor.run(Executor.java:429)
    Caused by: java.net.SocketTimeoutException: Read timed out
            at sun.reflect.GeneratedConstructorAccessor10056.newInstance(Unknown Source)
            at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
            at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
            at sun.net.www.protocol.http.HttpURLConnection$10.run(HttpURLConnection.java:1944)
            at sun.net.www.protocol.http.HttpURLConnection$10.run(HttpURLConnection.java:1939)
            at java.security.AccessController.doPrivileged(Native Method)
            at sun.net.www.protocol.http.HttpURLConnection.getChainedException(HttpURLConnection.java:1938)
            at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1508)
            at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1492)
            at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:480)
            at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:347)
            at org.kohsuke.github.Requester.parse(Requester.java:602)
            ... 15 more
    Caused by: java.net.SocketTimeoutException: Read timed out
            at java.net.SocketInputStream.socketRead0(Native Method)
            at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
            at java.net.SocketInputStream.read(SocketInputStream.java:171)
            at java.net.SocketInputStream.read(SocketInputStream.java:141)
            at sun.security.ssl.InputRecord.readFully(InputRecord.java:465)
            at sun.security.ssl.InputRecord.read(InputRecord.java:503)
            at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:983)
            at sun.security.ssl.SSLSocketImpl.readDataRecord(SSLSocketImpl.java:940)
            at sun.security.ssl.AppInputStream.read(AppInputStream.java:105)
            at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
            at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
            at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
            at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:735)
            at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:678)
            at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1587)
            at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1492)
            at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:480)
            at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:347)
            at org.kohsuke.github.Requester.parse(Requester.java:602)
            ... 13 more
    Finished: SUCCESS
    ```

    *Changes*

    Improve the merge script to check jenkins status before treating the checks as failed.

    Author: Sijie Guo <sijie@apache.org>

    Reviewers: Enrico Olivelli <eolivelli@gmail.com>

    This closes #1497 from sijie/improve_merge_script

6 weeks agoImprove merge script on checking the statuses of github checks
Sijie Guo [Thu, 7 Jun 2018 05:32:39 +0000 (22:32 -0700)] 
Improve merge script on checking the statuses of github checks

Descriptions of the changes in this PR:

*Motivation*

Sometimes Jenkins CI job might already succeed, however they failed on publishing the build status
to Github due to timeout (one example is attached below). In this case, we don't need to rerun the jenkins check again.

```
Setting status of 55c1d54f3293e5166c59f238afd3fb3d98c7eb67 to SUCCESS with url https://builds.apache.org/job/bookkeeper_precommit_client_tests/21/ and message: 'SUCCESS
 '
Using context: Jenkins: Client Tests
Could not update commit status of the Pull Request on GitHub.
org.kohsuke.github.HttpException: Server returned HTTP response code: -1, message: 'null' for URL: https://api.github.com/repos/apache/bookkeeper/statuses/55c1d54f3293e5166c59f238afd3fb3d98c7eb67
at org.kohsuke.github.Requester.parse(Requester.java:633)
at org.kohsuke.github.Requester.parse(Requester.java:631)
at org.kohsuke.github.Requester.parse(Requester.java:631)
at org.kohsuke.github.Requester.parse(Requester.java:594)
at org.kohsuke.github.Requester._to(Requester.java:272)
at org.kohsuke.github.Requester.to(Requester.java:234)
at org.kohsuke.github.GHRepository.createCommitStatus(GHRepository.java:1075)
at org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus.createCommitStatus(GhprbSimpleStatus.java:281)
at org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus.onBuildComplete(GhprbSimpleStatus.java:239)
at org.jenkinsci.plugins.ghprb.GhprbBuilds.onCompleted(GhprbBuilds.java:205)
at org.jenkinsci.plugins.ghprb.GhprbBuildListener.onCompleted(GhprbBuildListener.java:28)
at hudson.model.listeners.RunListener.fireCompleted(RunListener.java:211)
at hudson.model.Run.execute(Run.java:1769)
at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
at hudson.model.ResourceController.execute(ResourceController.java:97)
at hudson.model.Executor.run(Executor.java:429)
Caused by: java.net.SocketTimeoutException: Read timed out
at sun.reflect.GeneratedConstructorAccessor10056.newInstance(Unknown Source)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at sun.net.www.protocol.http.HttpURLConnection$10.run(HttpURLConnection.java:1944)
at sun.net.www.protocol.http.HttpURLConnection$10.run(HttpURLConnection.java:1939)
at java.security.AccessController.doPrivileged(Native Method)
at sun.net.www.protocol.http.HttpURLConnection.getChainedException(HttpURLConnection.java:1938)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1508)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1492)
at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:480)
at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:347)
at org.kohsuke.github.Requester.parse(Requester.java:602)
... 15 more
Caused by: java.net.SocketTimeoutException: Read timed out
at java.net.SocketInputStream.socketRead0(Native Method)
at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
at java.net.SocketInputStream.read(SocketInputStream.java:171)
at java.net.SocketInputStream.read(SocketInputStream.java:141)
at sun.security.ssl.InputRecord.readFully(InputRecord.java:465)
at sun.security.ssl.InputRecord.read(InputRecord.java:503)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:983)
at sun.security.ssl.SSLSocketImpl.readDataRecord(SSLSocketImpl.java:940)
at sun.security.ssl.AppInputStream.read(AppInputStream.java:105)
at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:735)
at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:678)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1587)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1492)
at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:480)
at sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:347)
at org.kohsuke.github.Requester.parse(Requester.java:602)
... 13 more
Finished: SUCCESS
```

*Changes*

Improve the merge script to check jenkins status before treating the checks as failed.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1497 from sijie/improve_merge_script

6 weeks agoRemove phrase trigger on post commit jobs
Sijie Guo [Wed, 6 Jun 2018 06:57:38 +0000 (23:57 -0700)] 
Remove phrase trigger on post commit jobs

Descriptions of the changes in this PR:

*Problem*

After enabling phrase trigger on precommit jobs to allow rerun individual precommit jobs, "retest this please" will run all the jobs including postcommit jobs.

*Solution*

Running postcommit jobs on pull requests doesn't make sense. This PR removes all the phrase trigger from postcommit jobs.

Additionally, fixes typos and also increases the timeout for integration tests.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1482 from sijie/fix_typoes

6 weeks agoFix checkstyle in EntryLogTest
Sijie Guo [Wed, 6 Jun 2018 06:55:34 +0000 (23:55 -0700)] 
Fix checkstyle in EntryLogTest

Descriptions of the changes in this PR:

*Problem*

PR #1465 introduces a checkstyle error, but somehow it wasn't caught by CI jobs.

*Solution*

Fix the checkstyle error and bring master back to normal.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Charan Reddy Guttapalem <reddycharan18@gmail.com>, Jia Zhai <None>, Enrico Olivelli <eolivelli@gmail.com>

This closes #1483 from sijie/fix_checkstyle_complains_entrylogtest

6 weeks agoUpgrade RocksDB to 5.13.1
Matteo Merli [Wed, 6 Jun 2018 06:53:27 +0000 (23:53 -0700)] 
Upgrade RocksDB to 5.13.1

Upgrade RocksDB version to include a fix for empty SSTs written by flushing with deleteRange() operations that cause assertion failures on DB open.

Description can be found at : facebook/rocksdb#2717

Author: Matteo Merli <mmerli@apache.org>

Reviewers: Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1466 from merlimat/upgrade-rocksdb

6 weeks agoMetrics for EntryLogManagerForEntryLogPerLedger.
cguttapalem [Tue, 5 Jun 2018 21:45:33 +0000 (14:45 -0700)] 
Metrics for EntryLogManagerForEntryLogPerLedger.

Descriptions of the changes in this PR:

Add counters/statsloggers for EntryLogManagerForEntryLogPerLedger.

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1460 from reddycharan/elmmetrics

6 weeks agoUpdate the new bookkeeper tools to use the new framework
Sijie Guo [Tue, 5 Jun 2018 21:39:52 +0000 (14:39 -0700)] 
Update the new bookkeeper tools to use the new framework

Descriptions of the changes in this PR:

*Motivation*

#1471  introduces a new tools framework based on jcommander. This PR is changing the new bookkeeper-tools to leverage that framework.

*Solution*

- Changed the related classes to use the new framework.
- Rename `BookKeeperCLI` to `BKCtl` and `bookkeeper-cli` to `bkctl`
- Use service loader to load command groups to allow better extensibility
- Rename command group `cluster` to `bookies`

*Results*

Example output of this change:

```
$ bin/bkctl
bkctl interacts and operates Apache BookKeeper clusters

Usage:  bkctl [flags] [command group] [commands]

Commands:

    bookie      Commands on operating a single bookie
    bookies     Commands on operating a cluster of bookies
    ledger      Commands on interacting with ledgers

    help        Display help information about it

Flags:

    -c, --conf
        Configuration file

    -u, --service-uri
        Service Uri

    -h, --help
        Display help information

Use "bkctl [command] --help" or "bkctl help [command]" for more information
about a command
```

*sub command*
```
$ bin/bkctl help bookie
Commands on operating a single bookie

Usage:  bkctl bookie [command] [command options]

Commands:

    lastmark        Print last log marker

    help            Display help information about it

Use "bkctl bookie [command] --help" or "bkctl bookie help [command]" for more
information about a command
```

*sub-sub command*

```
$ bin/bkctl bookie help lastmark
Print last log marker

Usage:  bkctl bookie lastmark [flags]

Flags:

    -h, --help
        Display help information
```

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1478 from sijie/bkctl_tools

6 weeks agoAppend ledgersMap when entrylog is removed from cache.
cguttapalem [Tue, 5 Jun 2018 20:04:38 +0000 (13:04 -0700)] 
Append ledgersMap when entrylog is removed from cache.

Descriptions of the changes in this PR:

In EntryLogManagerForEntryLogPerLedger when ledger-entrylog
entry is removed from cache, it will be moved to
rotatedEntryLogs list. Before moving it to rotatedEntryLogs,
ledgersMap should be appended.

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1465 from reddycharan/fixappendledgersmap

6 weeks agoImprove precommit build time
Sijie Guo [Tue, 5 Jun 2018 17:59:17 +0000 (10:59 -0700)] 
Improve precommit build time

Descriptions of the changes in this PR:

*Motivation*

The build/test time of `bookkeeper-server` is almost close to or more than 1 hr.

*Solution*

Split the precommit jobs into multiple jobs:

- *PR Validation* : run `apache-rat:check`, `checkstyle:check`, to ensure codestyle and license headers
- *Build (Java x)* : run `package` `spotbugs:check`, to ensure PR can be compiled and tested on different java environments. No spotbugs errors. `bookkeeper-server` tests are skipped.
- *Integration Tests* : run integration tests with docker environment
- *bookkeeper-server Tests* : tests `bookkeeper-server` on certain packages:
  - bookie: `org.apache.bookkeeper.bookie`
  - client: `org.apache.bookkeeper.client`
  - replication: `org.apache.bookkeeper.replication`
  - tls: `org.apache.bookkeeper.tls`
  - all others: all other tests in `bookkeeper-server` module

Each job can be triggered and skipped individually.

- *PR Validation* : "(re)run pr validation" | "" (unskippable)
- *Build (Java x)* : "(re)build java(8|9)" | "skip build java(8|9)"
  - "(re)build" to build on both java8 and java9
  - "skip build" to skip building jobs on both java8 and java9
- *Integration Tests* : "(re)run integration tests" | "skip integration tests"
- *bookkeeper-server tests* : "(re)run (bookie|client|replication|tls|remaining) tests" | "skip (bookie|client|replication|tls|remaining) tests"
  - "(re)run tests" to run all bookkeeper-server test jobs
  - "skip tests" to skip all bookkeeper-server test jobs

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1481 from sijie/improve_build_time

6 weeks agoThrottle integration tests to run 1 build per host
Sijie Guo [Tue, 5 Jun 2018 06:53:16 +0000 (23:53 -0700)] 
Throttle integration tests to run 1 build per host

Descriptions of the changes in this PR:

If running multiple integration tests build on same host, they will be conflicting on installing docker images. So throttle integration tests to run max 1 build per host.

See discussions at : http://mail-archives.apache.org/mod_mbox/pulsar-dev/201806.mbox/%3CCAJdLeK0nj9n-TKC4t43Nyc8bO9UUW17wM%2By_fF68EgFL6Fpo8w%40mail.gmail.com%3E

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1480 from sijie/disable_concurrent_builds

6 weeks agoAbstract the tools framework to allow merging multiple CLI tools together
Sijie Guo [Mon, 4 Jun 2018 20:30:43 +0000 (13:30 -0700)] 
Abstract the tools framework to allow merging multiple CLI tools together

Descriptions of the changes in this PR:

*Motivation*

There are multiple CLI tools spreading across multiple places, e.g. new bookkeeper cli, stream storage cli and dlog. There have similar implementations. It would be better to consolidate all these tools in one place `bookkeeper-tools`.

This is a PR to prepare moving `stream/cli` to be part of bookkeeper-tools.

*Solution*

- Abstract the CLI logic in bookkeeper-tools into a simple tools framework that can be reused in a extensible way to unify multiple tools together.
- organize the tools module into tools/framework and tools/all

*Result*

Example output of the tool using this framework is listed as below:

```
$ bin/bkctl
bkctl interacts and operates Apache BookKeeper clusters

Usage:  bkctl [flags] [command group] [commands]

Commands:

    bookie          Commands on operating a single bookie
    bookies         Commands on operating a cluster of bookies
    cluster         Commands on administrating bookkeeper clusters
    ledger          Commands on interacting with ledgers
    namespace       Commands on operating namespaces
    table           Commands on interacting with tables
    tables          Commands on operating tables

    help            Display help information about it

Flags:

    -c, --conf
        Configuration file

    -n, --namespace
        Namespace scope to run commands (only valid for table service for now)

    -u, --service-uri
        Service Uri

    -h, --help
        Display help information

Use "bkctl [command] --help" or "bkctl help [command]" for more information
about a command
```

*result from help sub-command*

```
$ bin/bkctl help table
Commands on interacting with tables

Usage:  bkctl table [command] [command options]

Commands:

    get         Get key/value pair from a table
    inc         Increment the amount of a key in a table
    put         Put key/value pair to a table

    help        Display help information about it

Use "bkctl table [command] --help" or "bkctl table help [command]" for more
information about a command
```

*result from help sub-sub-command*

```
$ bin/bkctl table help inc
Increment the amount of a key in a table

Usage:  bkctl table inc [flags] <table> <key> <amount>

Flags:

    -h, --help
        Display help information
```

Master Issue: #1000

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1471 from sijie/tools_framework

6 weeks agoImprove precommit CI jobs on handling PRs made to branches
Sijie Guo [Mon, 4 Jun 2018 20:07:42 +0000 (13:07 -0700)] 
Improve precommit CI jobs on handling PRs made to branches

Descriptions of the changes in this PR:

*Motivation*

Current precommit CI jobs uses `master` as target branch for applying the pull requests.
However pull requests might be made against branches. e.g. #1469, precommit jobs can't run properly on those pull requests. This PR tends to address the problem.

*Solution*

Use `ghprbTargetBranch` as the build branch for precommit jobs.

Besides that, split the validation goals (e.g. checkstyle, apache-rat) from build/test goals (package, spotbugs), this reduces build time for different jdk versions and address problem when running checkstyle:check with other goals (e.g. deploy).

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1475 from sijie/run_precommit_jobs_over_target_branches

6 weeks agoImprove merge script to merge PRs based on branches
Sijie Guo [Mon, 4 Jun 2018 20:00:04 +0000 (13:00 -0700)] 
Improve merge script to merge PRs based on branches

Descriptions of the changes in this PR:

*Motivation*

Merge script doesn't handle well on handling PRs based on branches. So committers ended up
using merge button to merge PRs. so we lose a lot of informations such as descriptions, reviewers
in the commit message.

*Solution*

This PR fixes the problems on handling merging PRs based on branches.

This allows us merging #1469 using merge script.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1477 from sijie/merge_pull_request_from_branch

6 weeks agoSupport build on JDK10
Enrico Olivelli [Mon, 4 Jun 2018 13:46:33 +0000 (15:46 +0200)] 
Support build on JDK10

- switch to "javac -h" in circe-checksum instead of 'javah' because in JDK10 javah has been dropped (see http://openjdk.java.net/jeps/313)
- add openjdk10 to Travis-CI

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1470 from eolivelli/nar-jdk10

6 weeks agoISSUE #1472: [TABLE SERVICE] TestStorageClientBuilder.testBuildClientInvalidNamespace...
Sijie Guo [Mon, 4 Jun 2018 07:57:09 +0000 (00:57 -0700)] 
ISSUE #1472: [TABLE SERVICE] TestStorageClientBuilder.testBuildClientInvalidNamespaceName failed

Descriptions of the changes in this PR:

*Problem*

 #1457 changed the validation of namespace/stream name. so "-" is a valid character.

*Solution*

Fix the test.

Master Issue: #1472

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1473 from sijie/fix_storage_client_builder, closes #1472

7 weeks agoIssue #1467: build failing with MavenReportException: Error while generating Javadoc...
Andrey Yegorov [Sat, 2 Jun 2018 05:08:46 +0000 (22:08 -0700)] 
Issue #1467: build failing with MavenReportException: Error while generating Javadoc. (#1468)

7 weeks agoIssue #1409: Added server side backpressure (@bug W-3651831@) (#1410)
Andrey Yegorov [Fri, 1 Jun 2018 23:28:17 +0000 (16:28 -0700)] 
Issue #1409: Added server side backpressure (@bug W-3651831@) (#1410)

(@bug W-3651831@) backpressure: server-side backpressure

7 weeks ago[CI] Run integration tests concurrently if possible
Sijie Guo [Fri, 1 Jun 2018 16:42:20 +0000 (09:42 -0700)] 
[CI] Run integration tests concurrently if possible

Descriptions of the changes in this PR:

Execute integration tests concurrently if necessary.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1463 from sijie/concurrent_builds

7 weeks agoISSUE #1455: Revert ": Unable to process suppressions file during checkstyle execution"
Sijie Guo [Fri, 1 Jun 2018 16:32:57 +0000 (09:32 -0700)] 
ISSUE #1455: Revert ": Unable to process suppressions file during checkstyle execution"

Descriptions of the changes in this PR:

This reverts commit 4bc021a438ca1e5acff23f7e3cb6cdb6984c1066 to address the issue we saw in #1461

Related Issue: #1456

This fixes #1461

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1462 from sijie/fix_maven_builds, closes #1455

7 weeks agoISSUE #1458: [CI] Fix bookkeeper-seed CI Job
Sijie Guo [Fri, 1 Jun 2018 16:30:49 +0000 (09:30 -0700)] 
ISSUE #1458: [CI] Fix bookkeeper-seed CI Job

Descriptions of the changes in this PR:

*Motivation*

Issue #1454 introduced a typo, which cause the seed job can not parse groovy DSL files.

*Solution*

Fix the typo.

Master Issue: #1458

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1464 from sijie/fix_job_seed, closes #1458

7 weeks agoBP-33: Move releasing official docker images out of main repo
Sijie Guo [Fri, 1 Jun 2018 06:26:04 +0000 (23:26 -0700)] 
BP-33: Move releasing official docker images out of main repo

Descriptions of the changes in this PR:

*Motivation*

Current bookkeeper docker images are auto-built by apache docker account. However it becomes problematic in the release process:

Docker autobuild uses release tag for labeling the versions for docker images. But the Dockerfile can only be updated after a release is successfully made. So we have to retag a release after a release, in order to update Dockerfile to build the docker images.

Master Issue: #1449

Author: Sijie Guo <sijie@apache.org>

Reviewers: Ivan Kelly <ivank@apache.org>, Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1450 from sijie/official_docker_images

7 weeks ago[TABLE SERVICE] cleanup : provide unified service uri for resolving service endpoints
Sijie Guo [Fri, 1 Jun 2018 06:24:02 +0000 (23:24 -0700)] 
[TABLE SERVICE] cleanup : provide unified service uri for resolving service endpoints

Descriptions of the changes in this PR:

*Motivation*

Currently there are multiple settings in `StorageClientSettings` on configuring how to resolve
service endpoints to find the services. It is a bit inconvinient and confusing

*Solution*

Provide a class `ServiceURI` for parsing service uri to retrieve common informations including:

- serviceName
- serviceInfos
- serviceUser
- serviceHosts
- servicePath

It extends the java.net.URI and does parsing and validation in the same place. Then it can be used
by the grpc name resolver to resolve service endpoints.

Also update dlog to use it for resolving its namespace uri.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1459 from sijie/name_resolver

7 weeks agoMake bookkeeper-server module buildable on jdk10
Enrico Olivelli [Thu, 31 May 2018 08:45:59 +0000 (10:45 +0200)] 
Make bookkeeper-server module buildable on jdk10

Upgrade lombok and javadoc plugin to latest versions.
This way it is possible to work on most of the modules using JDK10.

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1453 from eolivelli/fix/build-jdk10

7 weeks ago[TABLE SERVICE] cleanup : cleanup protobuf definitions
Sijie Guo [Thu, 31 May 2018 06:26:08 +0000 (23:26 -0700)] 
[TABLE SERVICE] cleanup : cleanup protobuf definitions

Descriptions of the changes in this PR:

*Motivation*

There are some unused fields and some fields that were not renamed correctly from col to namespace.

*Solution*

This is a cleanup PR to remove unused fields and rename "col" to "ns" or "namespace".

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1457 from sijie/cleanup_fields

7 weeks ago[TABLE SERVICE] remove StorageContainerRequest and StorageContainerResponse
Sijie Guo [Wed, 30 May 2018 17:18:15 +0000 (10:18 -0700)] 
[TABLE SERVICE] remove StorageContainerRequest and StorageContainerResponse

Descriptions of the changes in this PR:

*Motivation*

 #1448 uses reverse proxy for serving storage container requests and responses. so the `StorageContainerRequest` and `StorageContainerResponse`
 become redundant.

*Solution*

removes `StorageContainerRequest` and `StorageContainerResponse`

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1452 from sijie/remove_storage_container_request_response

7 weeks agoIssue 1455: Unable to process suppressions file during checkstyle execution
Sijie Guo [Wed, 30 May 2018 11:14:32 +0000 (13:14 +0200)] 
Issue 1455: Unable to process suppressions file during checkstyle execution

Descriptions of the changes in this PR:

*Motivation*

Failed to run `mvn clean apache-rat:check checkstyle:check install -DskipTests`

```
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0:check (default-cli) on project bookkeeper-stats-api: Failed during checkstyle execution: Unable to process suppressions file location: bookkeeper/suppressions.xml: Cannot create file-based resource:invalid block type -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :bookkeeper-stats-api
```

*Solution*

- Make `buildtools` not inherit from bookkeeper root pom.
- Make bookkeeper root pom includes `buildtools` as a `provided` dependency for each module.

Master Issue: #1455
Related Issues: #1435

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1456 from sijie/fix_ci_failure

7 weeks agoUpdate release procedure to use candidate tags for voting
Sijie Guo [Wed, 30 May 2018 07:56:40 +0000 (00:56 -0700)] 
Update release procedure to use candidate tags for voting

Descriptions of the changes in this PR:

*Motivation*

We used final release tag during release voting procedure. However there might be changes
happen between different candidate votes. So we ended up retagging the release tag for
different votes.

*Solution*

Use a candidate tag for voting. After a release is approved, create the final release tag.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Ivan Kelly <ivank@apache.org>, Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1451 from sijie/update_release_guid

7 weeks agoAllow CI to publish a snapshot version with gitsha information
Sijie Guo [Wed, 30 May 2018 07:55:27 +0000 (00:55 -0700)] 
Allow CI to publish a snapshot version with gitsha information

Descriptions of the changes in this PR:

*Motivation*

Currently bookkeeper publishes a snapshot release nightly to maven artifacts. It provides a convenient solution for people to try out new bookkeeper releases as soon as the changes land in master branch. However sometime it is also problematic when you reference this snapshot version in other projects, subsequent changes in bookkeeper might potentially break those projects.

*Solution*

Update the CI to provide a mechanism to publish a snapshot version with gitsha information. so it can publish a snapshot version that is pinned to the given gitsha. so the projects can safely use a snapshot version for development without worrying bookkeeper changes break it.

*NOTES*

The mechanism is a manual operation. It should be used when it is needed.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1454 from sijie/publish_snapshot_with_gitsha

7 weeks ago[TABLE SERVICE] use grpc reverse proxy to serve storage container requests
Sijie Guo [Tue, 29 May 2018 01:29:09 +0000 (18:29 -0700)] 
[TABLE SERVICE] use grpc reverse proxy to serve storage container requests

Descriptions of the changes in this PR:

*Motivation*

#1430 introduced client interceptor and grpc reverse proxy for storage container requests. This PR is the subsequent change to leverage client interceptor and reverse proxy for storage container requests.

*Changes*

**Client Changes**

Changed `StorageContainerChannel` to add client interceptor to stamp storage container id into the the requests' metadata.

**Server Changes**

- moved the stream storage logic out of storage containers to a `service` package. the main logic will be kept in `RangeStoreService` and `RangeStoreServiceFactory`.
- make the storage container logic for hosting `StorageContainerService`.
- Each storage container will run an inprogress grpc server for serving the grpc services registered to the container and provide an `channel` for accessing those grpc service.
- Changed the server to use reverse proxy for serving/proxying storage container requests.

*NOTE*

This change doesn't directly remove `StorageContainerRequest` and `StorageContainerResponse`.  It would be done in a subsequent change.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1448 from sijie/storage_container_channel

8 weeks agoInclude stream modules in bookkeeper distributions only when `-Dstream` is specified
Sijie Guo [Mon, 28 May 2018 05:57:22 +0000 (22:57 -0700)] 
Include stream modules in bookkeeper distributions only when `-Dstream` is specified

Descriptions of the changes in this PR:

*Motivation*

stream modules are only built when `-Dstream` is specified. so if we want to include
binary distribution, `-Dstream` is needed. This updates those modules with profiles
that enabled when `-Dstream` is specified.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1447 from sijie/fix_stream_module

8 weeks agoAdd printreplicationworkerid option to ListUnderreplicatedCmd.
cguttapalem [Mon, 28 May 2018 05:56:33 +0000 (22:56 -0700)] 
Add printreplicationworkerid option to ListUnderreplicatedCmd.

Descriptions of the changes in this PR:

Add printreplicationworkerid option to ListUnderreplicatedCmd.
This helps in knowing who is holding lock on UnderReplicated ledger,
and diagnosing that ReplicationWorker if we are seeing any issues in
rereplication.

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1446 from reddycharan/printreplicationworkerid

8 weeks ago[TABLE] mark TableClientTest & TableClientSimpleTest as FlakyTest
Sijie Guo [Sat, 26 May 2018 03:52:50 +0000 (20:52 -0700)] 
[TABLE] mark TableClientTest & TableClientSimpleTest as FlakyTest

Descriptions of the changes in this PR:

*Motivation*

These two integration tests are flaky on shutting down. Mark them as `FlakyTest` to make CI stable while investigating the issue.

*Changes*

Move `FlakyTest` annotation from bookkeeper-server test jar to bookkeeper-common test jar. So it can be used across multiple modules.

At the same time, remove redundant `FlakyTest` annotation at distributedlog-core module.

Related Issue: #1440

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1445 from sijie/mark_two_integration_tests_as_flaky

8 weeks agoUpdate bookkeeper dependencies : netty/protobuf/grpc
Sijie Guo [Fri, 25 May 2018 22:02:03 +0000 (15:02 -0700)] 
Update bookkeeper dependencies : netty/protobuf/grpc

Descriptions of the changes in this PR:

*Motivation*

The netty version in bookkeeper is a bit dated than the version in pulsar.
Hence the grpc version is limited to the version that use same netty version.
It causes issues at pulsar using bookkeeper, because pulsar has been using
a newer version for longer time. This causes conflicts when pulsar client and
grpc are used together without proper shading.

*Solution*

Upgrade netty/protobuf/grpc dependencies.

- update netty from `4.1.12` to `4.1.22`, which is the one grpc is using and closer to the one that pulsar is using.
- update grpc from `1.5.0` to `1.12.0`, which is using netty `4.1.22` and protobuf `3.5.1`.
- update protobuf from `3.4.0` to `3.5.1`

Related Issue: apache/pulsar#1844

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>, Matteo Merli <mmerli@apache.org>

This closes #1441 from sijie/bump_grpc_version

8 weeks agoProvide the flag to allow ignoring startup failures on loading extra server components
Sijie Guo [Fri, 25 May 2018 18:10:22 +0000 (11:10 -0700)] 
Provide the flag to allow ignoring startup failures on loading extra server components

Descriptions of the changes in this PR:

Addressed the comments in apache/bookkeeper#1420

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1426 from sijie/flag_to_allow_silent_failures

8 weeks agoISSUE #1442: Assembly descriptor contains a filesystem-root relative reference '/'
Sijie Guo [Fri, 25 May 2018 17:52:13 +0000 (10:52 -0700)] 
ISSUE #1442: Assembly descriptor contains a filesystem-root relative reference '/'

Descriptions of the changes in this PR:

*Motivation*

Warnings on buidling bookkeeper dist packages. See #1442.

*Solution*

Remove "/" from the relative reference.

See https://lonelycoding.com/maven-assembly-plugin-warning-the-assembly-descriptor-contains-a-filesystem-root-relative-reference/ for details.

Master Issue: #1442

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1443 from sijie/address_assembly_warnings, closes #1442

8 weeks agoFix checkstyle warnings
Sijie Guo [Fri, 25 May 2018 17:49:57 +0000 (10:49 -0700)] 
Fix checkstyle warnings

Descriptions of the changes in this PR:

*Motivation*

There were two concurrent PRs merged at the same time.

apache/bookkeeper#1435 (checkstyle changes) was merged before apache/bookkeeper#1430. so checkstyle warnings
introduced by apache/bookkeeper#1430 were not caught by any CI jobs.

*Solution*

Address the checkstyle warnings introduced by apache/bookkeeper#1430

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1444 from sijie/fix_checkstyle_issue

8 weeks ago[TABLE SERVICE] client interceptor and storage container grpc proxy
Sijie Guo [Fri, 25 May 2018 08:02:06 +0000 (01:02 -0700)] 
[TABLE SERVICE] client interceptor and storage container grpc proxy

Descriptions of the changes in this PR:

This is a subsequent change after apache/bookkeeper#1428.

*Motivation*

Current almost every grpc requests are wrapped into `StorageContainerRequest` and their responses
are wrapped into `StorageContainerResponse`. It makes things a bit complicated on adding new grpc
services.

*Changes*

To simplify things, this PR introduces two functionalities for simplifying dispatching container requests/responses.

1) *StorageContainerClientInterceptor*: A grpc `ClientInterceptor` that stamps container information (currently is `scId`) into the requests' metadata before sending the requests to the wire.

2) A simple grpc reverse proxy to dispatch grpc requests to the channels provided by a `ChannelFinder`.

*Tests*

1. Existing unit tests covered client interceptor changes.
2. Introduced a `stream-storage-tests-common` module to include common classes that would be used for testing.
3. Introduced a `PingPongService` for testing reverse proxy : unary/client-streaming/server-streaming/bidi-streaming.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1430 from sijie/interceptor_container_requests

8 weeks agoIssue #1434: Checkstyle is not executed at some modules
Sijie Guo [Fri, 25 May 2018 07:58:26 +0000 (00:58 -0700)] 
Issue #1434: Checkstyle is not executed at some modules

Descriptions of the changes in this PR:

*Motivation*

Checkstyle was supposed to run at `validate` phase for every modules. However checkstyle is not running at some modules.
This introduces inconsistency between modules.

*Changes*

- Fix Checkstyle warnings.
- Remove `checkstyle` plugins from modules, only leave it at root pom file or modules that have overrides. This improves default build time.
- Add `checkstyle:check` to CI jobs.

*NOTES*

`microbenchmarks` has too many checkstyle volations. so skip checkstyle for this PR.

Master Issue: #1434

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1435 from sijie/checkstyle_tests, closes #1434

8 weeks agoUse maven-exec-plugin to get the maven project version for CI jobs
Sijie Guo [Thu, 24 May 2018 22:48:59 +0000 (15:48 -0700)] 
Use maven-exec-plugin to get the maven project version for CI jobs

Descriptions of the changes in this PR:

*Motivation*

The current approach to get project version isn't reliable on CI environment. It might have garbage output (e.g. exceptions).

*Solution*

Changed to use `exec-maven-plugin` to echo `${project.vesion}`. It provides a more reliable approach to get version on CI environments.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1437 from sijie/fix_getversion_problem

8 weeks agoISSUE #1438: BookKeeperVerifierMain: fix typo for drain_timeout argument
Samuel Just [Thu, 24 May 2018 22:45:47 +0000 (15:45 -0700)] 
ISSUE #1438: BookKeeperVerifierMain: fix typo for drain_timeout argument

(bug W-5025785)
Signed-off-by: Samuel Just <sjustsalesforce.com>
Author: Samuel Just <sjust@salesforce.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>, Venkateswararao Jujjuri (JV) <None>

This closes #1439 from athanatos/forupstream/wip-1438, closes #1438

8 weeks agoAllow overriding `redirectTestOutputToFile` with environment settings
Sijie Guo [Thu, 24 May 2018 15:36:11 +0000 (08:36 -0700)] 
Allow overriding `redirectTestOutputToFile` with environment settings

Descriptions of the changes in this PR:

Some modules (e.g. `tests/integration`) are allowed to override `redirectTestOutputToFile` behavior; while some are disallowed.

This PR is to make all modules are allowed to override that setting.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1432 from sijie/make_redirectTestOutputToFile_configurable

8 weeks agoExclude `.vagrant` from apache-rat:check
Sijie Guo [Thu, 24 May 2018 15:35:33 +0000 (08:35 -0700)] 
Exclude `.vagrant` from apache-rat:check

Descriptions of the changes in this PR:

*Motivation*

apache/bookkeeper#1401 provides a `Vagrantfile` for running integration tests on linux vm.
A `.vagrant` directory will be left under `dev` directory, which it will fail `apache-rat:check`.

*Solution*

Exclude `.vagrant` directory from apache-rat check

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1431 from sijie/fix_apache_rat_check

8 weeks agoFix bookkeeper post commit CI
Sijie Guo [Wed, 23 May 2018 22:42:02 +0000 (15:42 -0700)] 
Fix bookkeeper post commit CI

Descriptions of the changes in this PR:

*Motivation*

apache/bookkeeper#1422 includes stream storage integration tests in tests/integration.
so we need to include `-Dstream` on building the tests.

*Solution*

Update the bookkeeper post commit CI jobs to include `-Dstream`

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1429 from sijie/fix_post_commit_ci

8 weeks ago[TABLE SERVICE] Move grpc services from server module to storage module
Sijie Guo [Wed, 23 May 2018 17:56:58 +0000 (10:56 -0700)] 
[TABLE SERVICE] Move grpc services from server module to storage module

Descriptions of the changes in this PR:

*Motivation*

Current almost every grpc requests are wrapped into `StorageContainerRequest` and their responses
are wrapped into `StorageContainerResponse`. It makes things a bit complicated on adding new grpc
services.

To simplify things, we can use grpc ClientInterceptor to stamp container information (e.g. scId)
into the request metadata and write a grpc service registry to take the container information from
request metadata and dispatch requests to containers.

In order to achieve it, we need to move the grpc services to storage container.

*Solution*

This PR moves grpc services from server module to storage module, so we can simplify the wire protocols.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1428 from sijie/move_grpc_service_to_storage

8 weeks agoPublish bookkeeper dev image to docker hub
Sijie Guo [Wed, 23 May 2018 17:56:04 +0000 (10:56 -0700)] 
Publish bookkeeper dev image to docker hub

Descriptions of the changes in this PR:

*Motivation*

We built a `bookkeeper-current` image that reflects latest master for integration tests. However currently we don't publish this docker image to any docker registry. So it is inconvenient to test latest master in dockerized environments.

*Solution*

- Add a dev script to publish `bookkeeper-current` image to configured docker registry specified at `DOCKER_REGISTRY`
- Update the nightly snapshot job to publish generated `bookkeeper-current` image to docker hub `apachebookkeeper`
- Add `bin/standalone` script to use `docker-compose` to launch a 3-nodes bookkeeper cluster locally. The script generates docker-compose.yml and handles mounting local volumes and port forwarding. so developers can use that for quick development.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1427 from sijie/push_current_docker_image

2 months ago[TABLE SERVICE] Move integration tests under `stream/tests/integration` to `tests...
Sijie Guo [Wed, 23 May 2018 02:11:16 +0000 (19:11 -0700)] 
[TABLE SERVICE] Move integration tests under `stream/tests/integration` to `tests/integration/cluster`

Descriptions of the changes in this PR:

The original integration tests were written based a non-dockerized standalone stream cluster. Moved them to use
the dockerized integration test framework. So all the integration tests are actually testing the table service run as part of bookies.

This change is based on #1422 .
a371ff2 is the change in this PR to be reviewed.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1423 from sijie/move_more_stream_it_tests

2 months agoISSUE #1415: [DLOG] org.apache.distributedlog.fs.TestDLFileSystem fails on CI
Enrico Olivelli [Wed, 23 May 2018 02:09:08 +0000 (19:09 -0700)] 
ISSUE #1415: [DLOG] org.apache.distributedlog.fs.TestDLFileSystem fails on CI

Use a generated tempDir for DL FS tests instead of using an hard coded /tmp path

Fix for #1415

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1417 from eolivelli/fix/no-tmpdir-fsdl, closes #1415

2 months ago[TABLE SERVICE] start table service as an extra component of bookie
Sijie Guo [Tue, 22 May 2018 18:43:31 +0000 (11:43 -0700)] 
[TABLE SERVICE] start table service as an extra component of bookie

Descriptions of the changes in this PR:

*Motivation*

table service was built over bookkeeper ledgers. it is an extension to bookies' functionalities, so it is much convenient to start the service as one additional component with bookie, just like how we start `AutoRecovery` along with bookie.

*Solution*

- include `stream/server` module as part of bookkeeper server/all binary distributions
- abstract `StorageServer` to allow controlling whether to start bookie or not
- provide a ServerLifecycleComponent of `StorageServer`, so it can be used as an extra component of bookie

*Tests*

- improve the integration tests to start table service as part of bookie containers
- move `LocationClientTest` to container based integration tests

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1422 from sijie/start_table_service

2 months agoBP-14 WriteFlag DEFERRED_SYNC Client Side Implementation
Enrico Olivelli [Tue, 22 May 2018 14:05:07 +0000 (16:05 +0200)] 
BP-14 WriteFlag DEFERRED_SYNC Client Side Implementation

Introduce implementation of WriteFlag#DEFERRED_SYNC on LedgerHandle (purely client-side).

Tests are only on the client-side (with Mockito), there is no implementation on the bookie side. Most significant tests will come with the force() API, at this point we can only test that we have not broken the LAC advance mechanism.

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: Sijie Guo <sijie@apache.org>, Venkateswararao Jujjuri (JV)

This closes #853 from eolivelli/bp14-deferred-force-write-client-side

2 months agoAllow bookie to start if failed to load extra server components
Sijie Guo [Tue, 22 May 2018 08:43:46 +0000 (01:43 -0700)] 
Allow bookie to start if failed to load extra server components

Descriptions of the changes in this PR:

*Motivation*

`extraServerComponents` provides the flexibility to allow bookie to start with extra server components.
It acts as a plugin mechanism to extend the functionality of bookies. However it is okay to allow bookie
start up when failed to load extra server components.

This is a change for allowing starting table service as extra components in bookies.

*Solution*

Catch the exception and log it when failed to load extra server components.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1420 from sijie/allow_start_bookie

2 months agoBuild stream modules for integration tests
Sijie Guo [Tue, 22 May 2018 07:27:46 +0000 (00:27 -0700)] 
Build stream modules for integration tests

Descriptions of the changes in this PR:

*Motivation*

apache/bookkeeper#1422 adds table service as part of integration tests. so we need to build stream modules
in order to run integration tests for apache/bookkeeper#1422

*Solution*

Add "-Dstream" in the maven commands for `precommit-integrationtests` CI job.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1424 from sijie/compile_stream_module

2 months agoProvide a util method to run functions with metadata client driver
Sijie Guo [Tue, 22 May 2018 05:39:40 +0000 (22:39 -0700)] 
Provide a util method to run functions with metadata client driver

Descriptions of the changes in this PR:

*Motivation*

Currently `MetadataDrivers` provides util methods to run functions with metadata bookie driver.
It is convinient to provide a util method to run functions with metadata client driver as well.

*Solution*

Provide a util method `runFunctionWithMetadataClientDriver` to run functions with metadata client driver.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1421 from sijie/utils_to_run_client_driver

2 months agoFix dev/check-binary-license on macOS
Sijie Guo [Tue, 22 May 2018 05:38:52 +0000 (22:38 -0700)] 
Fix dev/check-binary-license on macOS

Descriptions of the changes in this PR:

*Motivation*

`dev/check-binary-license` is used for checking whether LICENSE files are updated to reflect the dependencies included in the distribution packages.
However this script doesn't work on macOS, which makes the development on macOS inconvinient.

*Solution*

The change removes `--wildcards` from tar command. The option only works on linux machines.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1418 from sijie/fix_check_license_script_master

2 months agoIssue #570: EntryLogManagerForEntryLogPerLedger implementation
cguttapalem [Tue, 22 May 2018 02:02:35 +0000 (19:02 -0700)] 
Issue #570: EntryLogManagerForEntryLogPerLedger implementation

Descriptions of the changes in this PR:

This is < sub-task6  > of Issue #570

introducing EntryLogManagerForEntryLogPerLedger, which is the
implementation of EntryLogManager for entrylog per ledger
feature.

Master Issue: #570

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Andrey Yegorov <None>, Sijie Guo <sijie@apache.org>

This closes #1391 from reddycharan/entrylogmanagerentrylogperledger, closes #570

2 months agoProvide zookeeper startup script
Sijie Guo [Mon, 21 May 2018 21:54:52 +0000 (14:54 -0700)] 
Provide zookeeper startup script

Descriptions of the changes in this PR:

*Motivation*

ZooKeeper is a dependency of bookkeeper shipped along with bookkeeper dist package. However we never provide any script to startup zookeeper in the bookkeeper binary package and never test the zookeeper version shipped along with bookkeeper

*Solution*

- add zookeeper in `bin/bookeeper` scripts to start zookeeper using bookkeeper scripts
- refactor bookkeeper docker scripts to start bookie/zookeeper/standalone
- use zk adminPort for health check, since 4letters commands are explicitly disabled at 3.5

*Tests*

Existing integration tests test the docker scripts and the bash script changes to start zookeeper.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1416 from sijie/package_zookeeper

2 months agoBookies should be from different racks in a Writequorum.
cguttapalem [Mon, 21 May 2018 21:53:43 +0000 (14:53 -0700)] 
Bookies should be from different racks in a Writequorum.

Descriptions of the changes in this PR:

Ideally RackAwareEnsemblePlacementPolicy should try to select
bookies from different racks for a write quorum. So in a
WriteQuorum, bookies should be from WriteQuorum number of racks,
provided there are sufficient number of available racks and
bookies.

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Sijie Guo <sijie@apache.org>

This closes #1397 from reddycharan/configracks

2 months agoAdd a docker based `BookKeeperClusterTestBase` for failure related integration tests
Sijie Guo [Fri, 18 May 2018 19:49:31 +0000 (12:49 -0700)] 
Add a docker based `BookKeeperClusterTestBase` for failure related integration tests

Descriptions of the changes in this PR:

*Motivation*

Currently we don't have any failure related testing for table service. Since we are using docker as the integration testing infrastructure,
It is better to use container for those failure testings, rather than going down the path as what we did before.

*Solution*

This change provides the basic test base for bookkeeper cluster using dockers. `BookKeeperClusterTestBase` provides the similar functionalities
to start/stop bookies as what we did in the unit test.

`bookkeeper/tests/containers` in `integration-tests-topologies` provides all the basic containers used for testing.
`tests/integration/cluster` and `tests/integration/topologies` provides the test base for writing tests using dockerized bookkeeper cluster.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1412 from sijie/docker_cluster_dev

2 months agoAdd a test docker image for current version only
Sijie Guo [Thu, 17 May 2018 08:39:04 +0000 (01:39 -0700)] 
Add a test docker image for current version only

Descriptions of the changes in this PR:

*Motivation*

The current test images don't use any scripts that shipped with official docker image.
We basically don't have any test coverage on the official docker image we shipped for each release.

*Solution*

Introduce a `bookkeeper-current` image representing an image for latest master.

Use `apachebookkeeper/all-versions-image` for BC tests, use `apachebookkeeper/bookkeeper-current` as tests only needs latest master image.

Add an integration test to test standalone mode using the `bookkeeper-current` image

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1408 from sijie/docker_dev

2 months agoIssue #1405: ReplicationWorker should back-off retrying.
cguttapalem [Thu, 17 May 2018 08:38:20 +0000 (01:38 -0700)] 
Issue¬†#1405: ReplicationWorker should back-off retrying.

Descriptions of the changes in this PR:

ReplicationWorker should backoff replication
after threshold number of replication failures of a ledger.

Currently ReplicationWorker will do busy retrials if
replication is failed for a ledger, instead it should
backoff if replication had failed for threshold
number of times. This can be done by deferring
releasing of underreplicated lock by
'lockReleaseOfFailedLedgerGracePeriod' amount
of time.

Master Issue: #1405

Author: cguttapalem <cguttapalem@salesforce.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #1406 from reddycharan/fixreplicator, closes #1405

2 months agoISSUE #1390 Ensemble change on delayed write error
JV Jujjuri [Thu, 17 May 2018 08:37:28 +0000 (01:37 -0700)] 
ISSUE #1390 Ensemble change on delayed write error

Error on delayed writes are dropped if the addEntry
is in complete state (ack quorum satisfied).
This change records the delayed write failure and forces
ensemble change onthe next write. This saves from having
extended under replicated status on the ledger and also
avoids unnecessary build up at PCBC channel.

Signed-off-by: Venkateswararao Jujjuri (JV) <vjujjurisalesforce.com>
Descriptions of the changes in this PR:

(PR description content here)...

Master Issue: #<master-issue-number>

Author: JV Jujjuri <vjujjuri@salesforce.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1395 from jvrao/bk-issue-1390-delayedWriteError, closes #1390

2 months agoRefactor bookkeeper bash scripts and move dlog script to root bin directory
Sijie Guo [Thu, 17 May 2018 04:00:42 +0000 (21:00 -0700)] 
Refactor bookkeeper bash scripts and move dlog script to root bin directory

Descriptions of the changes in this PR:

*Motivation*

Since 4.7, we have moved bash scripts and configuration to the root directory. However the scripts and configurations for dlog and table-service modules are still in their own modules. It is inconvenient and confusing. This change mainly refactor current bookkeeper script to make it reusable for other modules.

*Code Change*

The main changes are:

- abstract the common logic in `bin/bookkeeper` to `bin/common.sh`. These common logics include:
   * common definitions on environment variables, such jvm settings, bk conf, log4j conf, classpath and such.
   * common functions can be reusable, such as find jars, add maven dependencies.
- simplify `bin/bookkeeper` and `bin/bookkeeper-cli` by reusing `bin/common.sh`
- remove `stream/distributedlog/core/bin/dlog` to `bin/dlog` and simplify it by reusing `bin/common.sh`

*Tests*

Most of the changes in this PR are tests to ensure this script refactor is done correctly.

- introduced a module `tests/scripts` for testing all the bash scripts in bookkeeper project. This module uses [shUnit2](https://github.com/kward/shunit2) for testing bash scripts. This gives a good test coverage on `bin/common.sh`.

- add a few CLI smoketests under `tests/integration/smoke` to smoke test all CLI tools, including `bin/bookkeeper shell`, `bin/bookkeeper-cli` and `bin/dlog`. This makes sure all the CLI scripts work well after refactor.

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1407 from sijie/include_dlog_library

2 months agoISSUE #1403: ArrayIndexOutOfBoundsException is thrown on readLastAddConfirmedAndEntry
Sijie Guo [Tue, 15 May 2018 18:12:54 +0000 (11:12 -0700)] 
ISSUE #1403: ArrayIndexOutOfBoundsException is thrown on readLastAddConfirmedAndEntry

Descriptions of the changes in this PR:

*Motivation*

There are two bugs in `ReadLastAddConfirmedAndEntry`:

1) a regression was introduced by #657. the long poll read op is attempting to long-poll reading lac. since lac is stored across ensemble, so the retry logic assumes it will attempt over all the bookies in the ensemble. however #657 use a `write-quorum-size` write set for tracking those attempts. this leads to ArrayIndexOutOfBoundsException reported at #1403. The integrate tests added in this PR can easily reproduce this issue.

2) there was a bug on retry logic when a ledger whose ensemble size is larger than write quorum size. when this happens, it will claim lac is not advanced prior to attempt the bookie in the ensemble. so the client will never know lac is advanced if using long poll reads on such ledgers. The integrate tests added in this PR can also catch this issue.

disclaim: twitter uses long poll reads but never uses `ensembleSize > writeQuorumSize`. so this is not a problem for dlog users.

*Solution*

- introduce a `getWriteSetForLongPoll` call that uses `ensembleSize` for building the write set. this would address problem 1)
- fix the assignment of `numEmptyResponsesAllowed`, so the long poll reads can work with `ensembleSize > writeQuorumSize`
- add integration tests for long polling reads

at the same time, also add an integration test for normal tailing reads with

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1404 from sijie/longpoll_tests, closes #1403

2 months agoUse isEmpty method instead of size check
Ali Ahmed [Tue, 15 May 2018 11:55:36 +0000 (13:55 +0200)] 
Use isEmpty method instead of size check

In HTTP ListLedgerService use is Empty instead of size check.

Author: Ali Ahmed <alahmed.se@gmail.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1371 from aahmed-se/minorRefactor