[GitHub] [zookeeper] anmolnar commented on issue #855: ZOOKEEPER-3310: Add metrics for prep processor

2019-05-08 Thread GitBox
anmolnar commented on issue #855: ZOOKEEPER-3310: Add metrics for prep processor
URL: https://github.com/apache/zookeeper/pull/855#issuecomment-490754714
 
 
   Got it. The counter of processed requested (cnt_)PREP_PROCESS_TIME is 
updated in the `run()` method _after_ `pRequest()`.
   `pRequest()` calls nextRequestProcessor.process() which is mocked to update 
the latch for synchronization. As a consequence the test could end before the 
counter is updated.
   
   To fix this I suggest to move the update of the counter inside the 
pRequest() call before `nextRequestProcessor.process(). It will be more 
accurate anyway.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (ZOOKEEPER-3358) Make Snappy The Default Snapshot Compression Algorithm

2019-05-08 Thread Fangmin Lv (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836100#comment-16836100
 ] 

Fangmin Lv commented on ZOOKEEPER-3358:
---

[~belugabehr] When we contributed that back, most of our prod ensembles are 
using snappy, it's quite helpful. But keep in mind, if user is storing binary 
data like gzipped data, then the compress ratio could be very small and it 
actually wastes CPU cycle to do the compress/decompress.

This is why we keep the default behavior as before since we don't know what 
users are storing, and it may change the behavior.

> Make Snappy The Default Snapshot Compression Algorithm
> --
>
> Key: ZOOKEEPER-3358
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3358
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: server
>Affects Versions: 3.6.0
>Reporter: David Mollitor
>Priority: Major
> Fix For: 3.6.0
>
>
> Now that SnapShots are compressed, thanks to [ZOOKEEPER-3179], make the 
> default algorithm Snappy compression.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ZOOKEEPER-3352) Use LevelDB For Backend

2019-05-08 Thread Fangmin Lv (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16836096#comment-16836096
 ] 

Fangmin Lv commented on ZOOKEEPER-3352:
---

Had an intern done some prototype with me by building the ZooKeeper on RocksDB, 
the intention is to store more data, use incremental snapshot to reduce the 
cost with LSM, reduce the GC effort and data loading time, etc. It also avoids 
fuzzy snapshot which is pretty error-prone and caused lots of consistency bugs 
in ZK. But it's likely won't give us the same throughput as what we have now 
with in memory data tree and append only txn file. 

Currently, we only moved snapshot to RocksDB, the plan is to remove the in 
memory data tree as well, but only cache things like children structure, 
metadata to retain txn prepare and read throughput, etc.

We tried to keep the file based txn file, but disable the WAL in RocksDB, since 
we cannot see obvious benefit for now.

This is still a prototype, and we didn't have time in the last year to spend 
more effort there, but it will be an interesting try to see how it goes when 
ZooKeeper meet other backend.

[~belugabehr] what's the status of this project on your side, is it a thought 
or you already have something?

 

 

> Use LevelDB For Backend
> ---
>
> Key: ZOOKEEPER-3352
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3352
> Project: ZooKeeper
>  Issue Type: New Feature
>  Components: server
>Reporter: David Mollitor
>Assignee: David Mollitor
>Priority: Critical
> Fix For: 4.0.0
>
>
> Use LevelDB for managing data stored in ZK (transaction logs and snapshots).
> https://stackoverflow.com/questions/6779669/does-leveldb-support-java



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [zookeeper] anmolnar commented on issue #855: ZOOKEEPER-3310: Add metrics for prep processor

2019-05-08 Thread GitBox
anmolnar commented on issue #855: ZOOKEEPER-3310: Add metrics for prep processor
URL: https://github.com/apache/zookeeper/pull/855#issuecomment-490750148
 
 
   @jhuan31 Looks like a new flaky test has been introduced by this patch:
   
   
https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk/512/testReport/junit/org.apache.zookeeper.server/PrepRequestProcessorMetricsTest/testPrepRequestProcessorMetrics/
   
   Would you please create a de-flaky jira, I'll try to look into the test 
today why it become flaky?
   Thanks.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


Re: why not disable sync API in libzkst.a?

2019-05-08 Thread Michael Han
Hi Shuxin,

What's the zk client version you are testing? Sync apis were removed from c
client in https://issues.apache.org/jira/browse/ZOOKEEPER-761, but it's
only done for 3.5.3 and master branch only. There was a discussion in JIRA
regarding whether or not to do the same for 3.4 branch which never
concluded. Feel free to reopen the JIRA and submit a patch for 3.4 branch.

On Mon, May 6, 2019 at 11:24 PM Shuxin Yang 
wrote:

> Hi,
>
>  I'm new to C API lib. I accidentally use single-thread lib (i.e.
> libzkst.a) and it took me quit a while before I realize all sync
> functions in the libzkst.a is not usable at all. I'm wondering why not
> just disable them in libztst.a. It would otherwise be pretty confusing
> and error-prone.
>
> Take zoo_get_children() as an example, it always return ZOK, and 0
> children.  This function in turn calls zoo_wget_children_() which is
> excerpted bellow. The wait_sync_completion() at line 3657 actually does
> nothing in the single-thread version, meaning this function simply
> return to the caller without waiting for the job done.
>
> Thanks
>
> Shuxin
>
>
> --
>
> 3646 static int zoo_wget_children_(...)
>
> 3650 struct sync_completion *sc = alloc_sync_completion();
>
> 
>
> 3655 rc= zoo_awget_children (zh, path, watcher, watcherCtx,
> SYNCHRONOUS_MARKER, sc);
> 3656 if(rc==ZOK){
> 3657 wait_sync_completion(sc);
> 3658 rc = sc->rc;
> 3659 if (rc == 0) {
> 3660 if (strings) {
> 3661 *strings = sc->u.strs2;
> 3662 } else {
> 3663 deallocate_String_vector(>u.strs2);
> 3664 }
> 3665 }
> 3666 }
> 3667 free_sync_completion(sc);
> 3668 return rc;
> 3669 }
>
> 
>
>
>


Re: [Suggestion] Use Co-authored-by in commit messages

2019-05-08 Thread Michael Han
>> My proposal is to use github's feature of Co-author
+1. The commit script would have to be updated to incorporate this feature.

>> if someone participate in the review of PR, no matter whether he/she is
a committer, we all need include his/her name
We already do this when commit a change so reviewers get credits as well
and we can keep it this way unless there is a better approach.


On Wed, May 8, 2019 at 11:23 AM Enrico Olivelli  wrote:

> Yes, it is a good idea to have a common practice for tracking the original
> author.
>
> IMHO this is up to the person who is picking up an old patch, it is his own
> responsibility.
>
> IIRC In Bookkeeper we keep the original author of the patch if the patch is
> a straight port from another private company fork with minimal changes.
>
> Having a Co-author is good from my side. I am not sure we can force it
>
> My 2cents
>
> Enrico
>
> Il mer 8 mag 2019, 19:31 Brian Nixon  ha
> scritto:
>
> > +1 to the idea of multiple authors, particularly for rescued code
> >
> > -1 to including all reviewers in the commit proper, this information is
> > easily enough found from poking at the mail archive where "original
> author"
> > requires studying a ticket on jira
> >
> > awesome idea!
> >
> >
> > On Wed, May 8, 2019 at 6:32 AM Norbert Kalmar
>  > >
> > wrote:
> >
> > > Sorry everyone for the multiple emails...
> > > So, I get your suggestion now Maoling, sorry for the confusion.
> > > We already indicate the reviewer if it's from an apache email, as it
> > looks
> > > to me. (Doesn't have to be ZooKeeper committer). We should add external
> > > emails as well.
> > >
> > > So I just clarified this with Andor, looks like this is a manual entry
> > (the
> > > names/emails itself) during the commit (script).
> > >
> > > Let's hear what others think :)
> > >
> > >
> > > On Wed, May 8, 2019 at 3:24 PM Norbert Kalmar 
> > > wrote:
> > >
> > > > Well, HBase does it for example, commits have a "Signed-off-by: ..."
> > > line.
> > > >
> > > > All right, votes on for co-author and signed-off-by :)
> > > >
> > > >
> > > >
> > > > On Wed, May 8, 2019 at 2:58 PM Norbert Kalmar 
> > > > wrote:
> > > >
> > > >> Thanks Maoling, I also think encouraging code review as well is a
> good
> > > >> idea, but, unfortunately I have a "but" :)
> > > >> I see two issues with including reviewers in the commit message.
> > > >> First, I don't think there is a method to automate this, although I
> > > think
> > > >> the commit script the committers are using can be modified to
> include
> > > it.
> > > >> Otherwise doing manually would complicate merging PRs for
> committers.
> > > >> My other, bigger issue is that there is nothing to track this
> > > >> information. At least I am not aware of anything. What I mean is
> > Github
> > > >> tracks authors of the commits. But what would we use the reviewers
> > > >> information? If you just want to check reviewers for whatever
> reason,
> > > there
> > > >> is a filter for that already on github, in the Pull Request view.
> And
> > > this
> > > >> would also make the commit message more "bloated".
> > > >>
> > > >> I'm not saying we shouldn't do this (not a -1 from my side), I just
> > have
> > > >> my concerns mentioned above.
> > > >>
> > > >> Is there any Apache project doing this? Just out of curiosity.
> > > >>
> > > >> Regards,
> > > >> Norbert
> > > >>
> > > >>
> > > >> On Wed, May 8, 2019 at 2:34 PM Justin Ling Mao <
> > > maoling199210...@sina.com>
> > > >> wrote:
> > > >>
> > > >>> +1,A very good Suggestion.Thanks Norbert.I also suggest about the
> > > >>> sign-off of the Reviewers' name.For the incentive, if someone
> > > participate
> > > >>> in the review of PR, no matter whether he/she is a committer, we
> all
> > > need
> > > >>> include his/her name?
> > > >>>
> > > >>> - Original Message -
> > > >>> From: Norbert Kalmar 
> > > >>> To: dev@zookeeper.apache.org
> > > >>> Subject: [Suggestion] Use Co-authored-by in commit messages
> > > >>> Date: 2019-05-08 17:36
> > > >>>
> > > >>> Hi Devs,
> > > >>> I've got this idea from HBase.
> > > >>> So: when there is a patch that is abandoned by its original author
> > for
> > > >>> any
> > > >>> reason, and it can no longer be merged, someone comes by, and asks
> to
> > > >>> continue to work on it. Usually the reply is to use the change
> freely
> > > or
> > > >>> no
> > > >>> reply at all. Either way, what people end up doing is a new pull
> > > request,
> > > >>> and (correct me if I'm wrong) we do not have a standardized method
> > how
> > > to
> > > >>> indicate, or even to indicate at all the original author.
> > > >>> My proposal is to use github's feature of Co-author, which is a way
> > of
> > > >>> attributing multiple authors of a given commit. See more details
> > here:
> > > >>>
> > > >>>
> > >
> >
> https://help.github.com/en/articles/creating-a-commit-with-multiple-authors
> > > >>> I wouldn't think this needs to be forced or anything on future PRs,
> > but
> > > >>> it's a 

Re: [ANNOUNCE] New ZooKeeper PMC member: Andor Molnar

2019-05-08 Thread Michael Han
Congratulations, Andor!

On Mon, Apr 29, 2019 at 9:10 AM Brian Nixon 
wrote:

> Awesome! Congrats, Andor, you've been doing great work!
>
>
> On Mon, Apr 29, 2019 at 7:31 AM Patrick Hunt  wrote:
>
> > Kudos Andor!
> >
> > Patrick
> >
> > On Mon, Apr 29, 2019 at 2:07 AM Enrico Olivelli 
> > wrote:
> >
> > > Congratulations!
> > >
> > > Enrico
> > >
> > > Il lun 29 apr 2019, 10:40 Norbert Kalmar  >
> > ha
> > > scritto:
> > >
> > > > Congratulations Andor, well deserved!
> > > >
> > > > On Mon, Apr 29, 2019 at 10:14 AM Flavio Junqueira 
> > > wrote:
> > > >
> > > > > The Apache ZooKeeper PMC recently invited Andor to join the PMC and
> > he
> > > > has
> > > > > accepted. Andor has made some significant contributions to the
> > project,
> > > > > including driving releases.  We are looking forward to even greater
> > > > > contributions from Andor now as part of the PMC.
> > > > >
> > > > > Congratulations and welcome aboard Andor!
> > > > >
> > > > > -Flavio on behalf of the Apache ZooKeeper PMC
> > > >
> > >
> >
>


[GitHub] [zookeeper] jhuan31 commented on issue #933: ZOOKEEPER-3379: De-flaky test in Quorum Packet Metrics

2019-05-08 Thread GitBox
jhuan31 commented on issue #933: ZOOKEEPER-3379: De-flaky test in Quorum Packet 
Metrics
URL: https://github.com/apache/zookeeper/pull/933#issuecomment-490712538
 
 
   CC @anmolnar This addresses your comment for JIRA 3305


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] lvfangmin commented on issue #922: ZOOKEEPER-3361: Add multi version of getChildren request

2019-05-08 Thread GitBox
lvfangmin commented on issue #922: ZOOKEEPER-3361: Add multi version of 
getChildren request
URL: https://github.com/apache/zookeeper/pull/922#issuecomment-490703229
 
 
   Is it cleaner to add a new getChildren batch API instead of adding a fake 
txn in multi-op? Any use case and benefit of having getChildren with other 
write ops? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (ZOOKEEPER-3364) Compile with strict options in order to check code quality

2019-05-08 Thread Hudson (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16835988#comment-16835988
 ] 

Hudson commented on ZOOKEEPER-3364:
---

SUCCESS: Integrated in Jenkins build Zookeeper-trunk-single-thread #348 (See 
[https://builds.apache.org/job/Zookeeper-trunk-single-thread/348/])
ZOOKEEPER-3364: Compile with strict options in order to check code (andor: rev 
999c834714aba859a96ba32d02e66fb63e70ab35)
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/CnxManagerTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/server/watch/WatcherOrBitSetTest.java
* (edit) 
zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatchManagerFactory.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/JUnit4ZKTestRunner.java
* (edit) pom.xml
* (edit) zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigMisconfigTest.java
* (edit) 
zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java
* (edit) zookeeper-contrib/pom.xml
* (edit) 
zookeeper-server/src/main/java/org/apache/zookeeper/metrics/impl/MetricsProviderBootstrap.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/MiniKdc.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/FourLetterWordsTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/LoadFromLogTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/InvalidSnapshotTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/SyncCallTest.java


> Compile with strict options in order to check code quality
> --
>
> Key: ZOOKEEPER-3364
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3364
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: build
>Affects Versions: 3.6.0
>Reporter: Enrico Olivelli
>Assignee: Enrico Olivelli
>Priority: Major
>  Labels: pull-request-available
> Fix For: 3.6.0
>
>  Time Spent: 7h 20m
>  Remaining Estimate: 0h
>
> In order to dismiss old QA tests based on ant (ZOOKEEPER-3351) we have to 
> enforce code quality by activating some falgs on javac at build time, namely:
>  
> {code:java}
> 
>    -Werror
>    -Xlint:deprecation
>    -Xlint:unchecked
>    
>    -Xpkginfo:always
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [zookeeper] lvfangmin commented on issue #843: ZOOKEEPER-3296: Explicitly closing the sslsocket when it failed handshake to prevent issue where peers cannot join quorum

2019-05-08 Thread GitBox
lvfangmin commented on issue #843: ZOOKEEPER-3296: Explicitly closing the 
sslsocket when it failed handshake to prevent issue where peers cannot join 
quorum
URL: https://github.com/apache/zookeeper/pull/843#issuecomment-490696659
 
 
   @anmolnar do you mind to take a look and merge this if it looks good to you?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (ZOOKEEPER-3364) Compile with strict options in order to check code quality

2019-05-08 Thread Hudson (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16835937#comment-16835937
 ] 

Hudson commented on ZOOKEEPER-3364:
---

FAILURE: Integrated in Jenkins build ZooKeeper-trunk #512 (See 
[https://builds.apache.org/job/ZooKeeper-trunk/512/])
ZOOKEEPER-3364: Compile with strict options in order to check code (andor: rev 
999c834714aba859a96ba32d02e66fb63e70ab35)
* (edit) zookeeper-contrib/pom.xml
* (edit) 
zookeeper-server/src/main/java/org/apache/zookeeper/metrics/impl/MetricsProviderBootstrap.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/server/watch/WatcherOrBitSetTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/SyncCallTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/CnxManagerTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/LoadFromLogTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/JUnit4ZKTestRunner.java
* (edit) zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/FourLetterWordsTest.java
* (edit) 
zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/InvalidSnapshotTest.java
* (edit) 
zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatchManagerFactory.java
* (edit) pom.xml
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/MiniKdc.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigMisconfigTest.java
* (edit) 
zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java


> Compile with strict options in order to check code quality
> --
>
> Key: ZOOKEEPER-3364
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3364
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: build
>Affects Versions: 3.6.0
>Reporter: Enrico Olivelli
>Assignee: Enrico Olivelli
>Priority: Major
>  Labels: pull-request-available
> Fix For: 3.6.0
>
>  Time Spent: 7h 20m
>  Remaining Estimate: 0h
>
> In order to dismiss old QA tests based on ant (ZOOKEEPER-3351) we have to 
> enforce code quality by activating some falgs on javac at build time, namely:
>  
> {code:java}
> 
>    -Werror
>    -Xlint:deprecation
>    -Xlint:unchecked
>    
>    -Xpkginfo:always
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ZOOKEEPER-3362) Create a simple checkstyle file

2019-05-08 Thread Hudson (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16835936#comment-16835936
 ] 

Hudson commented on ZOOKEEPER-3362:
---

FAILURE: Integrated in Jenkins build ZooKeeper-trunk #512 (See 
[https://builds.apache.org/job/ZooKeeper-trunk/512/])
ZOOKEEPER-3362: Create a simple checkstyle file (andor: rev 
cb36d5a95fca6bc098890bde517ba76901adf43a)
* (edit) zookeeper-assembly/src/main/assembly/source-package.xml
* (edit) .travis.yml
* (add) checkstyle.xml
* (edit) 
zookeeper-contrib/zookeeper-contrib-zooinspector/src/main/java/com/nitido/utils/toaster/Toaster.java
* (add) checkstyleSuppressions.xml
* (edit) zookeeper-client/zookeeper-client-c/pom.xml
* (edit) pom.xml


> Create a simple checkstyle file
> ---
>
> Key: ZOOKEEPER-3362
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3362
> Project: ZooKeeper
>  Issue Type: Task
>  Components: build
>Affects Versions: 3.6.0
>Reporter: Enrico Olivelli
>Assignee: Enrico Olivelli
>Priority: Major
>  Labels: pull-request-available
> Fix For: 3.6.0
>
>  Time Spent: 3h 50m
>  Remaining Estimate: 0h
>
> Create a basic checkstyle file, in order to cover the minimal check on 
> @author tags.
> This is needed in order to drop old ANT based precommit job (see 
> ZOOKEEPER-3351)
> We will not remove legacy checkstyle configuration file in 
> zookeeper-server/src/test/resources/checkstyle.xml because it is referred by 
> ANT build.xml files (even if we are not actually using that target).
> This task won't add a complete checkstyle configuration with usual checks 
> because it would imply almost a change at every .java in the codebase.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


ZooKeeper-trunk - Build # 512 - Failure

2019-05-08 Thread Apache Jenkins Server
See https://builds.apache.org/job/ZooKeeper-trunk/512/

###
## LAST 60 LINES OF THE CONSOLE 
###
[...truncated 189.07 KB...]
[junit] Running org.apache.zookeeper.test.SessionTrackerCheckTest in thread 
3
[junit] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.09 sec, Thread: 3, Class: org.apache.zookeeper.test.SessionTrackerCheckTest
[junit] Running org.apache.zookeeper.test.SessionUpgradeTest in thread 3
[junit] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
15.804 sec, Thread: 2, Class: org.apache.zookeeper.test.SessionTest
[junit] Running org.apache.zookeeper.test.StandaloneTest in thread 2
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.57 sec, Thread: 2, Class: org.apache.zookeeper.test.StandaloneTest
[junit] Running org.apache.zookeeper.test.StatTest in thread 2
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.136 sec, Thread: 2, Class: org.apache.zookeeper.test.StatTest
[junit] Running org.apache.zookeeper.test.StaticHostProviderTest in thread 2
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
18.167 sec, Thread: 3, Class: org.apache.zookeeper.test.SessionUpgradeTest
[junit] Running org.apache.zookeeper.test.StringUtilTest in thread 3
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.077 sec, Thread: 3, Class: org.apache.zookeeper.test.StringUtilTest
[junit] Tests run: 26, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.729 sec, Thread: 2, Class: org.apache.zookeeper.test.StaticHostProviderTest
[junit] Running org.apache.zookeeper.test.SyncCallTest in thread 3
[junit] Running org.apache.zookeeper.test.TruncateTest in thread 2
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.792 sec, Thread: 3, Class: org.apache.zookeeper.test.SyncCallTest
[junit] Running org.apache.zookeeper.test.WatchEventWhenAutoResetTest in 
thread 3
[junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
5.999 sec, Thread: 2, Class: org.apache.zookeeper.test.TruncateTest
[junit] Running org.apache.zookeeper.test.WatchedEventTest in thread 2
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.079 sec, Thread: 2, Class: org.apache.zookeeper.test.WatchedEventTest
[junit] Running org.apache.zookeeper.test.WatcherFuncTest in thread 2
[junit] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
2.142 sec, Thread: 2, Class: org.apache.zookeeper.test.WatcherFuncTest
[junit] Running org.apache.zookeeper.test.WatcherTest in thread 2
[junit] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
17.712 sec, Thread: 3, Class: 
org.apache.zookeeper.test.WatchEventWhenAutoResetTest
[junit] Running org.apache.zookeeper.test.X509AuthTest in thread 3
[junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.09 sec, Thread: 3, Class: org.apache.zookeeper.test.X509AuthTest
[junit] Running org.apache.zookeeper.test.ZkDatabaseCorruptionTest in 
thread 3
[junit] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
7.378 sec, Thread: 3, Class: org.apache.zookeeper.test.ZkDatabaseCorruptionTest
[junit] Running org.apache.zookeeper.test.ZooKeeperQuotaTest in thread 3
[junit] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.794 sec, Thread: 3, Class: org.apache.zookeeper.test.ZooKeeperQuotaTest
[junit] Running org.apache.zookeeper.util.PemReaderTest in thread 3
[junit] Tests run: 64, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
3.688 sec, Thread: 3, Class: org.apache.zookeeper.util.PemReaderTest
[junit] Running org.apache.jute.BinaryInputArchiveTest in thread 3
[junit] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.169 sec, Thread: 3, Class: org.apache.jute.BinaryInputArchiveTest
[junit] Running org.apache.jute.UtilsTest in thread 3
[junit] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.078 sec, Thread: 3, Class: org.apache.jute.UtilsTest
[junit] Running org.apache.jute.XmlInputArchiveTest in thread 3
[junit] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.171 sec, Thread: 3, Class: org.apache.jute.XmlInputArchiveTest
[junit] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
30.807 sec, Thread: 2, Class: org.apache.zookeeper.test.WatcherTest
[junit] Tests run: 109, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
344.371 sec, Thread: 1, Class: org.apache.zookeeper.test.NioNettySuiteTest
[junit] Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
255.115 sec, Thread: 4, Class: org.apache.zookeeper.test.ReconfigTest

fail.build.on.test.failure:

BUILD FAILED

[jira] [Commented] (ZOOKEEPER-3362) Create a simple checkstyle file

2019-05-08 Thread Hudson (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16835914#comment-16835914
 ] 

Hudson commented on ZOOKEEPER-3362:
---

FAILURE: Integrated in Jenkins build Zookeeper-trunk-single-thread #347 (See 
[https://builds.apache.org/job/Zookeeper-trunk-single-thread/347/])
ZOOKEEPER-3362: Create a simple checkstyle file (andor: rev 
cb36d5a95fca6bc098890bde517ba76901adf43a)
* (edit) pom.xml
* (edit) zookeeper-assembly/src/main/assembly/source-package.xml
* (add) checkstyleSuppressions.xml
* (edit) 
zookeeper-contrib/zookeeper-contrib-zooinspector/src/main/java/com/nitido/utils/toaster/Toaster.java
* (add) checkstyle.xml
* (edit) zookeeper-client/zookeeper-client-c/pom.xml
* (edit) .travis.yml


> Create a simple checkstyle file
> ---
>
> Key: ZOOKEEPER-3362
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3362
> Project: ZooKeeper
>  Issue Type: Task
>  Components: build
>Affects Versions: 3.6.0
>Reporter: Enrico Olivelli
>Assignee: Enrico Olivelli
>Priority: Major
>  Labels: pull-request-available
> Fix For: 3.6.0
>
>  Time Spent: 3h 50m
>  Remaining Estimate: 0h
>
> Create a basic checkstyle file, in order to cover the minimal check on 
> @author tags.
> This is needed in order to drop old ANT based precommit job (see 
> ZOOKEEPER-3351)
> We will not remove legacy checkstyle configuration file in 
> zookeeper-server/src/test/resources/checkstyle.xml because it is referred by 
> ANT build.xml files (even if we are not actually using that target).
> This task won't add a complete checkstyle configuration with usual checks 
> because it would imply almost a change at every .java in the codebase.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [zookeeper] anmolnar commented on issue #910: ZOOKEEPER-3364 Compile with strict options in order to check code quality

2019-05-08 Thread GitBox
anmolnar commented on issue #910: ZOOKEEPER-3364 Compile with strict options in 
order to check code quality
URL: https://github.com/apache/zookeeper/pull/910#issuecomment-490631264
 
 
   Committed to master branch. Thanks @eolivelli !


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Resolved] (ZOOKEEPER-3364) Compile with strict options in order to check code quality

2019-05-08 Thread Andor Molnar (JIRA)


 [ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3364?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Andor Molnar resolved ZOOKEEPER-3364.
-
Resolution: Fixed

Issue resolved by pull request 910
[https://github.com/apache/zookeeper/pull/910]

> Compile with strict options in order to check code quality
> --
>
> Key: ZOOKEEPER-3364
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3364
> Project: ZooKeeper
>  Issue Type: Improvement
>  Components: build
>Affects Versions: 3.6.0
>Reporter: Enrico Olivelli
>Assignee: Enrico Olivelli
>Priority: Major
>  Labels: pull-request-available
> Fix For: 3.6.0
>
>  Time Spent: 7h
>  Remaining Estimate: 0h
>
> In order to dismiss old QA tests based on ant (ZOOKEEPER-3351) we have to 
> enforce code quality by activating some falgs on javac at build time, namely:
>  
> {code:java}
> 
>    -Werror
>    -Xlint:deprecation
>    -Xlint:unchecked
>    
>    -Xpkginfo:always
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [zookeeper] asfgit closed pull request #910: ZOOKEEPER-3364 Compile with strict options in order to check code quality

2019-05-08 Thread GitBox
asfgit closed pull request #910: ZOOKEEPER-3364 Compile with strict options in 
order to check code quality
URL: https://github.com/apache/zookeeper/pull/910
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


Re: [Suggestion] Use Co-authored-by in commit messages

2019-05-08 Thread Enrico Olivelli
Yes, it is a good idea to have a common practice for tracking the original
author.

IMHO this is up to the person who is picking up an old patch, it is his own
responsibility.

IIRC In Bookkeeper we keep the original author of the patch if the patch is
a straight port from another private company fork with minimal changes.

Having a Co-author is good from my side. I am not sure we can force it

My 2cents

Enrico

Il mer 8 mag 2019, 19:31 Brian Nixon  ha scritto:

> +1 to the idea of multiple authors, particularly for rescued code
>
> -1 to including all reviewers in the commit proper, this information is
> easily enough found from poking at the mail archive where "original author"
> requires studying a ticket on jira
>
> awesome idea!
>
>
> On Wed, May 8, 2019 at 6:32 AM Norbert Kalmar  >
> wrote:
>
> > Sorry everyone for the multiple emails...
> > So, I get your suggestion now Maoling, sorry for the confusion.
> > We already indicate the reviewer if it's from an apache email, as it
> looks
> > to me. (Doesn't have to be ZooKeeper committer). We should add external
> > emails as well.
> >
> > So I just clarified this with Andor, looks like this is a manual entry
> (the
> > names/emails itself) during the commit (script).
> >
> > Let's hear what others think :)
> >
> >
> > On Wed, May 8, 2019 at 3:24 PM Norbert Kalmar 
> > wrote:
> >
> > > Well, HBase does it for example, commits have a "Signed-off-by: ..."
> > line.
> > >
> > > All right, votes on for co-author and signed-off-by :)
> > >
> > >
> > >
> > > On Wed, May 8, 2019 at 2:58 PM Norbert Kalmar 
> > > wrote:
> > >
> > >> Thanks Maoling, I also think encouraging code review as well is a good
> > >> idea, but, unfortunately I have a "but" :)
> > >> I see two issues with including reviewers in the commit message.
> > >> First, I don't think there is a method to automate this, although I
> > think
> > >> the commit script the committers are using can be modified to include
> > it.
> > >> Otherwise doing manually would complicate merging PRs for committers.
> > >> My other, bigger issue is that there is nothing to track this
> > >> information. At least I am not aware of anything. What I mean is
> Github
> > >> tracks authors of the commits. But what would we use the reviewers
> > >> information? If you just want to check reviewers for whatever reason,
> > there
> > >> is a filter for that already on github, in the Pull Request view. And
> > this
> > >> would also make the commit message more "bloated".
> > >>
> > >> I'm not saying we shouldn't do this (not a -1 from my side), I just
> have
> > >> my concerns mentioned above.
> > >>
> > >> Is there any Apache project doing this? Just out of curiosity.
> > >>
> > >> Regards,
> > >> Norbert
> > >>
> > >>
> > >> On Wed, May 8, 2019 at 2:34 PM Justin Ling Mao <
> > maoling199210...@sina.com>
> > >> wrote:
> > >>
> > >>> +1,A very good Suggestion.Thanks Norbert.I also suggest about the
> > >>> sign-off of the Reviewers' name.For the incentive, if someone
> > participate
> > >>> in the review of PR, no matter whether he/she is a committer, we all
> > need
> > >>> include his/her name?
> > >>>
> > >>> - Original Message -
> > >>> From: Norbert Kalmar 
> > >>> To: dev@zookeeper.apache.org
> > >>> Subject: [Suggestion] Use Co-authored-by in commit messages
> > >>> Date: 2019-05-08 17:36
> > >>>
> > >>> Hi Devs,
> > >>> I've got this idea from HBase.
> > >>> So: when there is a patch that is abandoned by its original author
> for
> > >>> any
> > >>> reason, and it can no longer be merged, someone comes by, and asks to
> > >>> continue to work on it. Usually the reply is to use the change freely
> > or
> > >>> no
> > >>> reply at all. Either way, what people end up doing is a new pull
> > request,
> > >>> and (correct me if I'm wrong) we do not have a standardized method
> how
> > to
> > >>> indicate, or even to indicate at all the original author.
> > >>> My proposal is to use github's feature of Co-author, which is a way
> of
> > >>> attributing multiple authors of a given commit. See more details
> here:
> > >>>
> > >>>
> >
> https://help.github.com/en/articles/creating-a-commit-with-multiple-authors
> > >>> I wouldn't think this needs to be forced or anything on future PRs,
> but
> > >>> it's a nice thing to have. And if someone sees an old patch, this
> could
> > >>> give more incentive to continue to work on it, knowing there's a
> > >>> guideline
> > >>> in the HowToContribute guide to credit him/her.
> > >>> I can update the guide at
> > >>>
> https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
> > if
> > >>> the reception is positive.
> > >>> Regards,
> > >>> Norbert
> > >>>
> > >>
> >
>


Re: [Suggestion] Use Co-authored-by in commit messages

2019-05-08 Thread Brian Nixon
+1 to the idea of multiple authors, particularly for rescued code

-1 to including all reviewers in the commit proper, this information is
easily enough found from poking at the mail archive where "original author"
requires studying a ticket on jira

awesome idea!


On Wed, May 8, 2019 at 6:32 AM Norbert Kalmar 
wrote:

> Sorry everyone for the multiple emails...
> So, I get your suggestion now Maoling, sorry for the confusion.
> We already indicate the reviewer if it's from an apache email, as it looks
> to me. (Doesn't have to be ZooKeeper committer). We should add external
> emails as well.
>
> So I just clarified this with Andor, looks like this is a manual entry (the
> names/emails itself) during the commit (script).
>
> Let's hear what others think :)
>
>
> On Wed, May 8, 2019 at 3:24 PM Norbert Kalmar 
> wrote:
>
> > Well, HBase does it for example, commits have a "Signed-off-by: ..."
> line.
> >
> > All right, votes on for co-author and signed-off-by :)
> >
> >
> >
> > On Wed, May 8, 2019 at 2:58 PM Norbert Kalmar 
> > wrote:
> >
> >> Thanks Maoling, I also think encouraging code review as well is a good
> >> idea, but, unfortunately I have a "but" :)
> >> I see two issues with including reviewers in the commit message.
> >> First, I don't think there is a method to automate this, although I
> think
> >> the commit script the committers are using can be modified to include
> it.
> >> Otherwise doing manually would complicate merging PRs for committers.
> >> My other, bigger issue is that there is nothing to track this
> >> information. At least I am not aware of anything. What I mean is Github
> >> tracks authors of the commits. But what would we use the reviewers
> >> information? If you just want to check reviewers for whatever reason,
> there
> >> is a filter for that already on github, in the Pull Request view. And
> this
> >> would also make the commit message more "bloated".
> >>
> >> I'm not saying we shouldn't do this (not a -1 from my side), I just have
> >> my concerns mentioned above.
> >>
> >> Is there any Apache project doing this? Just out of curiosity.
> >>
> >> Regards,
> >> Norbert
> >>
> >>
> >> On Wed, May 8, 2019 at 2:34 PM Justin Ling Mao <
> maoling199210...@sina.com>
> >> wrote:
> >>
> >>> +1,A very good Suggestion.Thanks Norbert.I also suggest about the
> >>> sign-off of the Reviewers' name.For the incentive, if someone
> participate
> >>> in the review of PR, no matter whether he/she is a committer, we all
> need
> >>> include his/her name?
> >>>
> >>> - Original Message -
> >>> From: Norbert Kalmar 
> >>> To: dev@zookeeper.apache.org
> >>> Subject: [Suggestion] Use Co-authored-by in commit messages
> >>> Date: 2019-05-08 17:36
> >>>
> >>> Hi Devs,
> >>> I've got this idea from HBase.
> >>> So: when there is a patch that is abandoned by its original author for
> >>> any
> >>> reason, and it can no longer be merged, someone comes by, and asks to
> >>> continue to work on it. Usually the reply is to use the change freely
> or
> >>> no
> >>> reply at all. Either way, what people end up doing is a new pull
> request,
> >>> and (correct me if I'm wrong) we do not have a standardized method how
> to
> >>> indicate, or even to indicate at all the original author.
> >>> My proposal is to use github's feature of Co-author, which is a way of
> >>> attributing multiple authors of a given commit. See more details here:
> >>>
> >>>
> https://help.github.com/en/articles/creating-a-commit-with-multiple-authors
> >>> I wouldn't think this needs to be forced or anything on future PRs, but
> >>> it's a nice thing to have. And if someone sees an old patch, this could
> >>> give more incentive to continue to work on it, knowing there's a
> >>> guideline
> >>> in the HowToContribute guide to credit him/her.
> >>> I can update the guide at
> >>> https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
> if
> >>> the reception is positive.
> >>> Regards,
> >>> Norbert
> >>>
> >>
>


[GitHub] [zookeeper] eolivelli closed pull request #918: ZOOKEEPER-3366: Pluggable metrics system for ZooKeeper - move remaining metrics to MetricsProvider

2019-05-08 Thread GitBox
eolivelli closed pull request #918: ZOOKEEPER-3366: Pluggable metrics system 
for ZooKeeper - move remaining metrics to MetricsProvider
URL: https://github.com/apache/zookeeper/pull/918
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] eolivelli opened a new pull request #918: ZOOKEEPER-3366: Pluggable metrics system for ZooKeeper - move remaining metrics to MetricsProvider

2019-05-08 Thread GitBox
eolivelli opened a new pull request #918: ZOOKEEPER-3366: Pluggable metrics 
system for ZooKeeper - move remaining metrics to MetricsProvider
URL: https://github.com/apache/zookeeper/pull/918
 
 
   Migrate all remaining metrics to MetricsProvider.
   We are introducing now Gauges which are callbacks to be called when the 
Provider needs to publish current values.
   As during the lifecycle of a ZK server process we can have several 
ZooKeeperServer instances, depending on the role of the current process, we 
have to clean up unused Gauges, this is different from the old approach in 
which in 4lw and on http admin  API we had hard coded metrics, with multiple 
'instanceof' conditions.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] eolivelli commented on issue #910: ZOOKEEPER-3364 Compile with strict options in order to check code quality

2019-05-08 Thread GitBox
eolivelli commented on issue #910: ZOOKEEPER-3364 Compile with strict options 
in order to check code quality
URL: https://github.com/apache/zookeeper/pull/910#issuecomment-490558063
 
 
   retest ant build


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] anmolnar commented on issue #909: ZOOKEEPER-3362 Create a simple checkstyle file

2019-05-08 Thread GitBox
anmolnar commented on issue #909: ZOOKEEPER-3362 Create a simple checkstyle file
URL: https://github.com/apache/zookeeper/pull/909#issuecomment-490542616
 
 
   Thanks, committed to master.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Resolved] (ZOOKEEPER-3362) Create a simple checkstyle file

2019-05-08 Thread Andor Molnar (JIRA)


 [ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3362?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Andor Molnar resolved ZOOKEEPER-3362.
-
Resolution: Fixed

Issue resolved by pull request 909
[https://github.com/apache/zookeeper/pull/909]

> Create a simple checkstyle file
> ---
>
> Key: ZOOKEEPER-3362
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3362
> Project: ZooKeeper
>  Issue Type: Task
>  Components: build
>Affects Versions: 3.6.0
>Reporter: Enrico Olivelli
>Assignee: Enrico Olivelli
>Priority: Major
>  Labels: pull-request-available
> Fix For: 3.6.0
>
>  Time Spent: 3.5h
>  Remaining Estimate: 0h
>
> Create a basic checkstyle file, in order to cover the minimal check on 
> @author tags.
> This is needed in order to drop old ANT based precommit job (see 
> ZOOKEEPER-3351)
> We will not remove legacy checkstyle configuration file in 
> zookeeper-server/src/test/resources/checkstyle.xml because it is referred by 
> ANT build.xml files (even if we are not actually using that target).
> This task won't add a complete checkstyle configuration with usual checks 
> because it would imply almost a change at every .java in the codebase.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] [zookeeper] asfgit closed pull request #909: ZOOKEEPER-3362 Create a simple checkstyle file

2019-05-08 Thread GitBox
asfgit closed pull request #909: ZOOKEEPER-3362 Create a simple checkstyle file
URL: https://github.com/apache/zookeeper/pull/909
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] anmolnar commented on issue #909: ZOOKEEPER-3362 Create a simple checkstyle file

2019-05-08 Thread GitBox
anmolnar commented on issue #909: ZOOKEEPER-3362 Create a simple checkstyle file
URL: https://github.com/apache/zookeeper/pull/909#issuecomment-490536056
 
 
   @eolivelli Why isn't the new checkstyle file not the same as the ant version?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] eolivelli commented on issue #909: ZOOKEEPER-3362 Create a simple checkstyle file

2019-05-08 Thread GitBox
eolivelli commented on issue #909: ZOOKEEPER-3362 Create a simple checkstyle 
file
URL: https://github.com/apache/zookeeper/pull/909#issuecomment-490538183
 
 
   @anmolnar thank you for checking this patch
   
   > @eolivelli Why isn't the new checkstyle file not the same as the ant 
version?
   This new file is very simple and it mostly checks only for "author" javadoc 
tags it is the same check we are doing in the precommit ANT based build.
   
   Current "checkstyle" file is not really used and we would have thousands of 
failures.
   It will be an hard work to make the code base compliant with such file (I 
can help doing it but it will take weeks and many patches)
   
   So my approach is to:
   - introduce a new checkstyle file (it will grow in the future)
   - do what is needed to be able to drop the ANT based precommit script
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


Re: [Suggestion] Use Co-authored-by in commit messages

2019-05-08 Thread Norbert Kalmar
Well, HBase does it for example, commits have a "Signed-off-by: ..." line.

All right, votes on for co-author and signed-off-by :)



On Wed, May 8, 2019 at 2:58 PM Norbert Kalmar  wrote:

> Thanks Maoling, I also think encouraging code review as well is a good
> idea, but, unfortunately I have a "but" :)
> I see two issues with including reviewers in the commit message.
> First, I don't think there is a method to automate this, although I think
> the commit script the committers are using can be modified to include it.
> Otherwise doing manually would complicate merging PRs for committers.
> My other, bigger issue is that there is nothing to track this information.
> At least I am not aware of anything. What I mean is Github tracks authors
> of the commits. But what would we use the reviewers information? If you
> just want to check reviewers for whatever reason, there is a filter for
> that already on github, in the Pull Request view. And this would also make
> the commit message more "bloated".
>
> I'm not saying we shouldn't do this (not a -1 from my side), I just have
> my concerns mentioned above.
>
> Is there any Apache project doing this? Just out of curiosity.
>
> Regards,
> Norbert
>
>
> On Wed, May 8, 2019 at 2:34 PM Justin Ling Mao 
> wrote:
>
>> +1,A very good Suggestion.Thanks Norbert.I also suggest about the
>> sign-off of the Reviewers' name.For the incentive, if someone participate
>> in the review of PR, no matter whether he/she is a committer, we all need
>> include his/her name?
>>
>> - Original Message -
>> From: Norbert Kalmar 
>> To: dev@zookeeper.apache.org
>> Subject: [Suggestion] Use Co-authored-by in commit messages
>> Date: 2019-05-08 17:36
>>
>> Hi Devs,
>> I've got this idea from HBase.
>> So: when there is a patch that is abandoned by its original author for any
>> reason, and it can no longer be merged, someone comes by, and asks to
>> continue to work on it. Usually the reply is to use the change freely or
>> no
>> reply at all. Either way, what people end up doing is a new pull request,
>> and (correct me if I'm wrong) we do not have a standardized method how to
>> indicate, or even to indicate at all the original author.
>> My proposal is to use github's feature of Co-author, which is a way of
>> attributing multiple authors of a given commit. See more details here:
>>
>> https://help.github.com/en/articles/creating-a-commit-with-multiple-authors
>> I wouldn't think this needs to be forced or anything on future PRs, but
>> it's a nice thing to have. And if someone sees an old patch, this could
>> give more incentive to continue to work on it, knowing there's a guideline
>> in the HowToContribute guide to credit him/her.
>> I can update the guide at
>> https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute if
>> the reception is positive.
>> Regards,
>> Norbert
>>
>


Re: [Suggestion] Use Co-authored-by in commit messages

2019-05-08 Thread Norbert Kalmar
Sorry everyone for the multiple emails...
So, I get your suggestion now Maoling, sorry for the confusion.
We already indicate the reviewer if it's from an apache email, as it looks
to me. (Doesn't have to be ZooKeeper committer). We should add external
emails as well.

So I just clarified this with Andor, looks like this is a manual entry (the
names/emails itself) during the commit (script).

Let's hear what others think :)


On Wed, May 8, 2019 at 3:24 PM Norbert Kalmar  wrote:

> Well, HBase does it for example, commits have a "Signed-off-by: ..." line.
>
> All right, votes on for co-author and signed-off-by :)
>
>
>
> On Wed, May 8, 2019 at 2:58 PM Norbert Kalmar 
> wrote:
>
>> Thanks Maoling, I also think encouraging code review as well is a good
>> idea, but, unfortunately I have a "but" :)
>> I see two issues with including reviewers in the commit message.
>> First, I don't think there is a method to automate this, although I think
>> the commit script the committers are using can be modified to include it.
>> Otherwise doing manually would complicate merging PRs for committers.
>> My other, bigger issue is that there is nothing to track this
>> information. At least I am not aware of anything. What I mean is Github
>> tracks authors of the commits. But what would we use the reviewers
>> information? If you just want to check reviewers for whatever reason, there
>> is a filter for that already on github, in the Pull Request view. And this
>> would also make the commit message more "bloated".
>>
>> I'm not saying we shouldn't do this (not a -1 from my side), I just have
>> my concerns mentioned above.
>>
>> Is there any Apache project doing this? Just out of curiosity.
>>
>> Regards,
>> Norbert
>>
>>
>> On Wed, May 8, 2019 at 2:34 PM Justin Ling Mao 
>> wrote:
>>
>>> +1,A very good Suggestion.Thanks Norbert.I also suggest about the
>>> sign-off of the Reviewers' name.For the incentive, if someone participate
>>> in the review of PR, no matter whether he/she is a committer, we all need
>>> include his/her name?
>>>
>>> - Original Message -
>>> From: Norbert Kalmar 
>>> To: dev@zookeeper.apache.org
>>> Subject: [Suggestion] Use Co-authored-by in commit messages
>>> Date: 2019-05-08 17:36
>>>
>>> Hi Devs,
>>> I've got this idea from HBase.
>>> So: when there is a patch that is abandoned by its original author for
>>> any
>>> reason, and it can no longer be merged, someone comes by, and asks to
>>> continue to work on it. Usually the reply is to use the change freely or
>>> no
>>> reply at all. Either way, what people end up doing is a new pull request,
>>> and (correct me if I'm wrong) we do not have a standardized method how to
>>> indicate, or even to indicate at all the original author.
>>> My proposal is to use github's feature of Co-author, which is a way of
>>> attributing multiple authors of a given commit. See more details here:
>>>
>>> https://help.github.com/en/articles/creating-a-commit-with-multiple-authors
>>> I wouldn't think this needs to be forced or anything on future PRs, but
>>> it's a nice thing to have. And if someone sees an old patch, this could
>>> give more incentive to continue to work on it, knowing there's a
>>> guideline
>>> in the HowToContribute guide to credit him/her.
>>> I can update the guide at
>>> https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute if
>>> the reception is positive.
>>> Regards,
>>> Norbert
>>>
>>


[jira] [Created] (ZOOKEEPER-3381) Add multi watchregistration support for multi getChildren

2019-05-08 Thread Peter Szecsi (JIRA)
Peter Szecsi created ZOOKEEPER-3381:
---

 Summary: Add multi watchregistration support for multi getChildren
 Key: ZOOKEEPER-3381
 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3381
 Project: ZooKeeper
  Issue Type: Improvement
  Components: java client, tests
Affects Versions: 3.6.0
Reporter: Peter Szecsi
Assignee: Peter Szecsi


Currently, the client API only supports to register one watch attached to one 
node for a single request. However, for complete support of the multi version 
of the {{GetChildren}} this functionality needs to be extended.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


Re: [Suggestion] Use Co-authored-by in commit messages

2019-05-08 Thread Norbert Kalmar
Thanks Maoling, I also think encouraging code review as well is a good
idea, but, unfortunately I have a "but" :)
I see two issues with including reviewers in the commit message.
First, I don't think there is a method to automate this, although I think
the commit script the committers are using can be modified to include it.
Otherwise doing manually would complicate merging PRs for committers.
My other, bigger issue is that there is nothing to track this information.
At least I am not aware of anything. What I mean is Github tracks authors
of the commits. But what would we use the reviewers information? If you
just want to check reviewers for whatever reason, there is a filter for
that already on github, in the Pull Request view. And this would also make
the commit message more "bloated".

I'm not saying we shouldn't do this (not a -1 from my side), I just have my
concerns mentioned above.

Is there any Apache project doing this? Just out of curiosity.

Regards,
Norbert


On Wed, May 8, 2019 at 2:34 PM Justin Ling Mao 
wrote:

> +1,A very good Suggestion.Thanks Norbert.I also suggest about the sign-off
> of the Reviewers' name.For the incentive, if someone participate in the
> review of PR, no matter whether he/she is a committer, we all need include
> his/her name?
>
> - Original Message -
> From: Norbert Kalmar 
> To: dev@zookeeper.apache.org
> Subject: [Suggestion] Use Co-authored-by in commit messages
> Date: 2019-05-08 17:36
>
> Hi Devs,
> I've got this idea from HBase.
> So: when there is a patch that is abandoned by its original author for any
> reason, and it can no longer be merged, someone comes by, and asks to
> continue to work on it. Usually the reply is to use the change freely or no
> reply at all. Either way, what people end up doing is a new pull request,
> and (correct me if I'm wrong) we do not have a standardized method how to
> indicate, or even to indicate at all the original author.
> My proposal is to use github's feature of Co-author, which is a way of
> attributing multiple authors of a given commit. See more details here:
> https://help.github.com/en/articles/creating-a-commit-with-multiple-authors
> I wouldn't think this needs to be forced or anything on future PRs, but
> it's a nice thing to have. And if someone sees an old patch, this could
> give more incentive to continue to work on it, knowing there's a guideline
> in the HowToContribute guide to credit him/her.
> I can update the guide at
> https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute if
> the reception is positive.
> Regards,
> Norbert
>


[jira] [Commented] (ZOOKEEPER-3301) Enforce the quota limit

2019-05-08 Thread David Mollitor (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16835582#comment-16835582
 ] 

David Mollitor commented on ZOOKEEPER-3301:
---

Also be sure to take a look at [ZOOKEEPER-3347]

> Enforce the quota limit
> ---
>
> Key: ZOOKEEPER-3301
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3301
> Project: ZooKeeper
>  Issue Type: Sub-task
>Reporter: maoling
>Priority: Major
>
> We need a complete quota feature, not just the printing the warning logs 
> which is a bit chicken ribs.
> [zk: localhost:2181(CONNECTED) 18] setquota -n 2 /quota_test
> [zk: localhost:2181(CONNECTED) 19] create /quota_test/child_1
> Created /quota_test/child_1
> [zk: localhost:2181(CONNECTED) 20] create /quota_test/child_2
> Created /quota_test/child_2
> [zk: localhost:2181(CONNECTED) 21] create /quota_test/child_3
> Created /quota_test/child_3
> look at the following logs:
> 2019-03-07 11:22:36,680 [myid:1] - WARN [SyncThread:0:DataTree@374] - Quota 
> exceeded: /quota_test count=3 limit=2
> 2019-03-07 11:22:41,861 [myid:1] - WARN [SyncThread:0:DataTree@374] - Quota 
> exceeded: /quota_test count=4 limit=2



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


Re: [Suggestion] Use Co-authored-by in commit messages

2019-05-08 Thread Justin Ling Mao
+1,A very good Suggestion.Thanks Norbert.I also suggest about the sign-off of 
the Reviewers' name.For the incentive, if someone participate in the review of 
PR, no matter whether he/she is a committer, we all need include his/her name?

- Original Message -
From: Norbert Kalmar 
To: dev@zookeeper.apache.org
Subject: [Suggestion] Use Co-authored-by in commit messages
Date: 2019-05-08 17:36

Hi Devs,
I've got this idea from HBase.
So: when there is a patch that is abandoned by its original author for any
reason, and it can no longer be merged, someone comes by, and asks to
continue to work on it. Usually the reply is to use the change freely or no
reply at all. Either way, what people end up doing is a new pull request,
and (correct me if I'm wrong) we do not have a standardized method how to
indicate, or even to indicate at all the original author.
My proposal is to use github's feature of Co-author, which is a way of
attributing multiple authors of a given commit. See more details here:
https://help.github.com/en/articles/creating-a-commit-with-multiple-authors
I wouldn't think this needs to be forced or anything on future PRs, but
it's a nice thing to have. And if someone sees an old patch, this could
give more incentive to continue to work on it, knowing there's a guideline
in the HowToContribute guide to credit him/her.
I can update the guide at
https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute if
the reception is positive.
Regards,
Norbert


[jira] [Updated] (ZOOKEEPER-3301) Enforce the quota limit

2019-05-08 Thread maoling (JIRA)


 [ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3301?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

maoling updated ZOOKEEPER-3301:
---
Summary: Enforce the quota limit  (was: do a hard constraint on the 
"setquota",make it can really control the data size and child counts of one 
node.)

> Enforce the quota limit
> ---
>
> Key: ZOOKEEPER-3301
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3301
> Project: ZooKeeper
>  Issue Type: Sub-task
>Reporter: maoling
>Priority: Major
>
> We need a complete quota feature, not just the printing the warning logs 
> which is a bit chicken ribs.
> [zk: localhost:2181(CONNECTED) 18] setquota -n 2 /quota_test
> [zk: localhost:2181(CONNECTED) 19] create /quota_test/child_1
> Created /quota_test/child_1
> [zk: localhost:2181(CONNECTED) 20] create /quota_test/child_2
> Created /quota_test/child_2
> [zk: localhost:2181(CONNECTED) 21] create /quota_test/child_3
> Created /quota_test/child_3
> look at the following logs:
> 2019-03-07 11:22:36,680 [myid:1] - WARN [SyncThread:0:DataTree@374] - Quota 
> exceeded: /quota_test count=3 limit=2
> 2019-03-07 11:22:41,861 [myid:1] - WARN [SyncThread:0:DataTree@374] - Quota 
> exceeded: /quota_test count=4 limit=2



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ZOOKEEPER-3301) do a hard constraint on the "setquota",make it can really control the data size and child counts of one node.

2019-05-08 Thread maoling (JIRA)


[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16835538#comment-16835538
 ] 

maoling commented on ZOOKEEPER-3301:


find another issue:

[zk: 127.0.0.1:2180(CONNECTED) 6] create /you
Created /you
[zk: 127.0.0.1:2180(CONNECTED) 7] create /you/quota
Created /you/quota
[zk: 127.0.0.1:2180(CONNECTED) 8] set /you/quota data
[zk: 127.0.0.1:2180(CONNECTED) 9] create /you/quota/data data
Created /you/quota/data
[zk: 127.0.0.1:2180(CONNECTED) 10] setquota -b 5 /you/quota
fuck---code:NONODE
fuck---code:NONODE
[zk: 127.0.0.1:2180(CONNECTED) 11] set /you/quota/data newdata

2019-05-08 20:07:45,027 [myid:] - WARN [SyncThread:0:NIOServerCnxn@699] - 
Unexpected exception. Destruction averted.
java.lang.NullPointerException
 at 
org.apache.jute.BinaryOutputArchive.writeRecord(BinaryOutputArchive.java:123)
 at 
org.apache.zookeeper.proto.SetDataResponse.serialize(SetDataResponse.java:41)
 at 
org.apache.jute.BinaryOutputArchive.writeRecord(BinaryOutputArchive.java:123)
 at org.apache.zookeeper.server.ServerCnxn.serializeRecord(ServerCnxn.java:119)
 at org.apache.zookeeper.server.ServerCnxn.serialize(ServerCnxn.java:145)
 at 
org.apache.zookeeper.server.NIOServerCnxn.sendResponse(NIOServerCnxn.java:696)
 at org.apache.zookeeper.server.ServerCnxn.sendResponse(ServerCnxn.java:112)
 at 
org.apache.zookeeper.server.FinalRequestProcessor.processRequest(FinalRequestProcessor.java:567)
 at 
org.apache.zookeeper.server.SyncRequestProcessor.flush(SyncRequestProcessor.java:184)
 at 
org.apache.zookeeper.server.SyncRequestProcessor.run(SyncRequestProcessor.java:115)

> do a hard constraint on the "setquota",make it can really control the data 
> size and child counts of one node.
> -
>
> Key: ZOOKEEPER-3301
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3301
> Project: ZooKeeper
>  Issue Type: Sub-task
>Reporter: maoling
>Priority: Major
>
> We need a complete quota feature, not just the printing the warning logs 
> which is a bit chicken ribs.
> [zk: localhost:2181(CONNECTED) 18] setquota -n 2 /quota_test
> [zk: localhost:2181(CONNECTED) 19] create /quota_test/child_1
> Created /quota_test/child_1
> [zk: localhost:2181(CONNECTED) 20] create /quota_test/child_2
> Created /quota_test/child_2
> [zk: localhost:2181(CONNECTED) 21] create /quota_test/child_3
> Created /quota_test/child_3
> look at the following logs:
> 2019-03-07 11:22:36,680 [myid:1] - WARN [SyncThread:0:DataTree@374] - Quota 
> exceeded: /quota_test count=3 limit=2
> 2019-03-07 11:22:41,861 [myid:1] - WARN [SyncThread:0:DataTree@374] - Quota 
> exceeded: /quota_test count=4 limit=2



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[Suggestion] Use Co-authored-by in commit messages

2019-05-08 Thread Norbert Kalmar
Hi Devs,

I've got this idea from HBase.
So: when there is a patch that is abandoned by its original author for any
reason, and it can no longer be merged, someone comes by, and asks to
continue to work on it. Usually the reply is to use the change freely or no
reply at all. Either way, what people end up doing is a new pull request,
and (correct me if I'm wrong) we do not have a standardized method how to
indicate, or even to indicate at all the original author.

My proposal is to use github's feature of Co-author, which is a way of
attributing multiple authors of a given commit. See more details here:
https://help.github.com/en/articles/creating-a-commit-with-multiple-authors

I wouldn't think this needs to be forced or anything on future PRs, but
it's a nice thing to have. And if someone sees an old patch, this could
give more incentive to continue to work on it, knowing there's a guideline
in the HowToContribute guide to credit him/her.

I can update the guide at
https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute if
the reception is positive.

Regards,
Norbert