[GitHub] zookeeper issue #626: ZOOKEEPER-3148 Add Kerberos tests for modern JDKs
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/626 @anmolnar @phunt @nkalmar Green light from CI Can you please take a look ? So that we can archive this 'problem' and provide quick answers to users ---
[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...
Github user ivmaykov commented on the issue:
https://github.com/apache/zookeeper/pull/184
I think as long as we keep `portUnification=false` there should not be
hangs/crashes. However, that means it's not possible to safely upgrade a
cluster from plaintext quorum to TLS quorum without downtime. @hanm mentioned
"mitigations" but there really isn't a way to mitigate the issues w/
`UnifiedServerSocket` in #184 (other than "don't use it"). One option is to
keep the unified server socket code, but don't parse the `portUnification`
option in `QuorumPeerConfig` so there is no way to use the feature. Or we could
document the issues and have a clear warning ("portUnification has known
problems and may cause your ensemble to enter a bad state that requires
reboots, use at your own risk"), and let people take the risk if they like.
One issue that we found is the >10% perf regression in plaintext mode when
the apache httpclient library dependency is added. We never figured out why it
caused the perf regression, but it could be a potential blocker. Unlike the
port unification bugs, this perf regression cannot be worked around - it was
present even when all the SSL features were disabled. The fix for that is small
and isolated to one file, so it would be pretty easy to backport to #184 if
desired.
Let me just do another pass over the differences between the two PRs and
see if anything else jumps out at me.
---
[jira] [Commented] (ZOOKEEPER-3098) Add additional server metrics
[
https://issues.apache.org/jira/browse/ZOOKEEPER-3098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16618304#comment-16618304
]
Hudson commented on ZOOKEEPER-3098:
---
SUCCESS: Integrated in Jenkins build ZooKeeper-trunk #195 (See
[https://builds.apache.org/job/ZooKeeper-trunk/195/])
ZOOKEEPER-3098: Add additional server metrics (hanm: rev
f4cbb689ad7bb03ca6da0d5de3ecb344165f379a)
* (edit) src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
* (edit) src/java/main/org/apache/zookeeper/server/ServerCnxn.java
* (edit) src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java
* (add) src/java/main/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java
* (edit) src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java
* (edit) src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
* (add) src/java/main/org/apache/zookeeper/server/ServerMetrics.java
* (edit) src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java
* (edit) src/java/main/org/apache/zookeeper/server/ZKDatabase.java
* (edit) src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
* (edit) src/java/main/org/apache/zookeeper/server/admin/Commands.java
* (add) src/java/main/org/apache/zookeeper/server/metric/Metric.java
* (edit) src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java
* (edit) src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
* (add) src/java/main/org/apache/zookeeper/server/metric/SimpleCounter.java
* (edit) src/java/main/org/apache/zookeeper/server/quorum/Leader.java
* (edit)
src/java/main/org/apache/zookeeper/server/quorum/UpgradeableSessionTracker.java
* (edit) src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
* (edit) src/java/main/org/apache/zookeeper/server/SessionTracker.java
* (edit) src/java/main/org/apache/zookeeper/server/ServerStats.java
* (edit) src/java/main/org/apache/zookeeper/server/quorum/Follower.java
* (edit) src/java/test/org/apache/zookeeper/server/ServerStatsTest.java
* (add) src/java/test/org/apache/zookeeper/server/ServerMetricsTest.java
* (edit) src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java
> Add additional server metrics
> -
>
> Key: ZOOKEEPER-3098
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3098
> Project: ZooKeeper
> Issue Type: Improvement
> Components: server
>Affects Versions: 3.6.0
>Reporter: Joseph Blomstedt
>Assignee: Joseph Blomstedt
>Priority: Major
> Labels: pull-request-available
> Fix For: 3.6.0
>
> Time Spent: 6h
> Remaining Estimate: 0h
>
> This patch adds several new server-side metrics as well as makes it easier to
> add new metrics in the future. This patch also includes a handful of other
> minor metrics-related changes.
> Here's a high-level summary of the changes.
> # This patch extends the request latency tracked in {{ServerStats}} to track
> {{read}} and {{update}} latency separately. Updates are any request that must
> be voted on and can change data, reads are all requests that can be handled
> locally and don't change data.
> # This patch adds the {{ServerMetrics}} logic and the related
> {{AvgMinMaxCounter}} and {{SimpleCounter}} classes. This code is designed to
> make it incredibly easy to add new metrics. To add a new metric you just add
> one line to {{ServerMetrics}} and then directly reference that new metric
> anywhere in the code base. The {{ServerMetrics}} logic handles creating the
> metric, properly adding the metric to the JSON output of the {{/monitor}}
> admin command, and properly resetting the metric when necessary. The
> motivation behind {{ServerMetrics}} is to make things easy enough that it
> encourages new metrics to be added liberally. Lack of in-depth
> metrics/visibility is a long-standing ZooKeeper weakness. At Facebook, most
> of our internal changes build on {{ServerMetrics}} and we have nearly 100
> internal metrics at this time – all of which we'll be upstreaming in the
> coming months as we publish more internal patches.
> # This patch adds 20 new metrics, 14 which are handled by {{ServerMetrics}}.
> # This patch replaces some uses of {{synchronized}} in {{ServerStats}} with
> atomic operations.
> Here's a list of new metrics added in this patch:
> - {{uptime}}: time that a peer has been in a stable
> leading/following/observing state
> - {{leader_uptime}}: uptime for peer in leading state
> - {{global_sessions}}: count of global sessions
> - {{local_sessions}}: count of local sessions
> - {{quorum_size}}: configured ensemble size
> - {{synced_observers}}: similar to existing `synced_followers` but for
> observers
> - {{fsynctime}}: time to fsync transaction log (avg/min/max)
> - {{snapshottime}}: time to write a snapshot (avg/min/max
ZooKeeper-trunk-windows-cmake - Build # 2941 - Still Failing
See https://builds.apache.org/job/ZooKeeper-trunk-windows-cmake/2941/ ### ## LAST 60 LINES OF THE CONSOLE ### [...truncated 10.60 KB...] [ivy:retrieve] confs: [javacc] [ivy:retrieve] found net.java.dev.javacc#javacc;5.0 in maven2 [ivy:retrieve] :: resolution report :: resolve 40ms :: artifacts dl 1ms - | |modules|| artifacts | | conf | number| search|dwnlded|evicted|| number|dwnlded| - | javacc | 1 | 0 | 0 | 0 || 1 | 0 | - [ivy:retrieve] :: retrieving :: org.apache.zookeeper#zookeeper [ivy:retrieve] confs: [javacc] [ivy:retrieve] 1 artifacts copied, 0 already retrieved (291kB/14ms) generate_jute_parser: [mkdir] Created dir: f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\build\jute_compiler\org\apache\jute\compiler\generated [ivy:artifactproperty] DEPRECATED: 'ivy.conf.file' is deprecated, use 'ivy.settings.file' instead [ivy:artifactproperty] :: loading settings :: file = f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\ivysettings.xml [move] Moving 1 file to f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\build\javacc\lib [javacc] Java Compiler Compiler Version 5.0 (Parser Generator) [javacc] (type "javacc" with no arguments for help) [javacc] Reading from file f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\zookeeper-jute\src\main\java\org\apache\jute\compiler\generated\rcc.jj . . . [javacc] File "TokenMgrError.java" does not exist. Will create one. [javacc] File "ParseException.java" does not exist. Will create one. [javacc] File "Token.java" does not exist. Will create one. [javacc] File "SimpleCharStream.java" does not exist. Will create one. [javacc] Parser generated successfully. jute: [javac] Compiling 39 source files to f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\build\classes [javac] warning: [options] bootstrap class path not set in conjunction with -source 1.8 [javac] 1 warning compile_jute_uptodate: compile_jute: [mkdir] Created dir: f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\src\java\generated [mkdir] Created dir: f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\zookeeper-client\zookeeper-client-c\generated [java] ../../../zookeeper-jute/src/main/resources/zookeeper.jute Parsed Successfully [java] ../../../zookeeper-jute/src/main/resources/zookeeper.jute Parsed Successfully [touch] Creating f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\src\java\generated\.generated BUILD SUCCESSFUL Total time: 7 seconds [ZooKeeper-trunk-windows-cmake] $ cmd /c call F:\jenkins\jenkins-slave\temp\jenkins7857438317240713576.bat f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake>cd src/c The system cannot find the path specified. f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake>cmake . CMake Error: The source directory "F:/jenkins/jenkins-slave/workspace/ZooKeeper-trunk-windows-cmake" does not appear to contain CMakeLists.txt. Specify --help for usage, or press the help button on the CMake GUI. f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake>cmake --build . Error: could not load cache f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake>exit 1 Build step 'Execute Windows batch command' marked build as failure Email was triggered for: Failure - Any Sending email for trigger: Failure - Any ### ## FAILED TESTS (if any) ## No tests ran.
[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...
Github user enixon commented on the issue: https://github.com/apache/zookeeper/pull/184 +1 for @anmolnar 's plan Afaik, with `portUnification=false` there shouldn't be any problems. @ivmaykov does this work for you? ---
[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/184 I like @anmolnar plan. To mitigate the stability concerns around `UnifiedServerSocket`, we can disable it by default. Unless I misunderstand something, when unified port is disabled via `portUnification=false`, there should not be any DOS or stability issues observed during your tests, is this correct? @ivmaykov Regarding remove port unification / unified server socket from this PR, that's definitely another approach, but consider we need add it anyway plus we have mitigations in place this would just create more work. Regarding concerns of the final release with this PR committed but not any of the #627 , I think it's a fair concern, but given we have the mitigation in place I think we can release in worst case (that this PR goes in but none of #627 ) - though realistically I am optimistic that we can get most if not all of the #627 in before stable 3.5 release. ---
[GitHub] zookeeper pull request #580: ZOOKEEPER-3098: Add additional server metrics
Github user asfgit closed the pull request at: https://github.com/apache/zookeeper/pull/580 ---
[jira] [Resolved] (ZOOKEEPER-3098) Add additional server metrics
[
https://issues.apache.org/jira/browse/ZOOKEEPER-3098?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Michael Han resolved ZOOKEEPER-3098.
Resolution: Fixed
Fix Version/s: 3.6.0
Issue resolved by pull request 580
[https://github.com/apache/zookeeper/pull/580]
> Add additional server metrics
> -
>
> Key: ZOOKEEPER-3098
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3098
> Project: ZooKeeper
> Issue Type: Improvement
> Components: server
>Affects Versions: 3.6.0
>Reporter: Joseph Blomstedt
>Assignee: Joseph Blomstedt
>Priority: Major
> Labels: pull-request-available
> Fix For: 3.6.0
>
> Time Spent: 5h 50m
> Remaining Estimate: 0h
>
> This patch adds several new server-side metrics as well as makes it easier to
> add new metrics in the future. This patch also includes a handful of other
> minor metrics-related changes.
> Here's a high-level summary of the changes.
> # This patch extends the request latency tracked in {{ServerStats}} to track
> {{read}} and {{update}} latency separately. Updates are any request that must
> be voted on and can change data, reads are all requests that can be handled
> locally and don't change data.
> # This patch adds the {{ServerMetrics}} logic and the related
> {{AvgMinMaxCounter}} and {{SimpleCounter}} classes. This code is designed to
> make it incredibly easy to add new metrics. To add a new metric you just add
> one line to {{ServerMetrics}} and then directly reference that new metric
> anywhere in the code base. The {{ServerMetrics}} logic handles creating the
> metric, properly adding the metric to the JSON output of the {{/monitor}}
> admin command, and properly resetting the metric when necessary. The
> motivation behind {{ServerMetrics}} is to make things easy enough that it
> encourages new metrics to be added liberally. Lack of in-depth
> metrics/visibility is a long-standing ZooKeeper weakness. At Facebook, most
> of our internal changes build on {{ServerMetrics}} and we have nearly 100
> internal metrics at this time – all of which we'll be upstreaming in the
> coming months as we publish more internal patches.
> # This patch adds 20 new metrics, 14 which are handled by {{ServerMetrics}}.
> # This patch replaces some uses of {{synchronized}} in {{ServerStats}} with
> atomic operations.
> Here's a list of new metrics added in this patch:
> - {{uptime}}: time that a peer has been in a stable
> leading/following/observing state
> - {{leader_uptime}}: uptime for peer in leading state
> - {{global_sessions}}: count of global sessions
> - {{local_sessions}}: count of local sessions
> - {{quorum_size}}: configured ensemble size
> - {{synced_observers}}: similar to existing `synced_followers` but for
> observers
> - {{fsynctime}}: time to fsync transaction log (avg/min/max)
> - {{snapshottime}}: time to write a snapshot (avg/min/max)
> - {{dbinittime}}: time to reload database – read snapshot + apply
> transactions (avg/min/max)
> - {{readlatency}}: read request latency (avg/min/max)
> - {{updatelatency}}: update request latency (avg/min/max)
> - {{propagation_latency}}: end-to-end latency for updates, from proposal on
> leader to committed-to-datatree on a given host (avg/min/max)
> - {{follower_sync_time}}: time for follower to sync with leader (avg/min/max)
> - {{election_time}}: time between entering and leaving election (avg/min/max)
> - {{looking_count}}: number of transitions into looking state
> - {{diff_count}}: number of diff syncs performed
> - {{snap_count}}: number of snap syncs performed
> - {{commit_count}}: number of commits performed on leader
> - {{connection_request_count}}: number of incoming client connection requests
> - {{bytes_received_count}}: similar to existing `packets_received` but
> tracks bytes
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] zookeeper issue #580: ZOOKEEPER-3098: Add additional server metrics
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/580 Got a [green build](https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2193/). Merging. ---
Success: ZOOKEEPER- PreCommit Build #2193
Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2193/ ### ## LAST 60 LINES OF THE CONSOLE ### [...truncated 81.47 MB...] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +0 tests included. The patch appears to be a documentation patch that doesn't require tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 core tests. The patch passed core unit tests. [exec] [exec] +1 contrib tests. The patch passed contrib unit tests. [exec] [exec] Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2193//testReport/ [exec] Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2193//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html [exec] Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2193//console [exec] [exec] This message is automatically generated. [exec] [exec] [exec] == [exec] == [exec] Adding comment to Jira. [exec] == [exec] == [exec] [exec] [exec] [exec] Error: No value specified for option "issue" [exec] Session logged out. Session was JSESSIONID=3F7C9FA472C9B0923B0025AE2FA13E0E. [exec] [exec] [exec] == [exec] == [exec] Finished build. [exec] == [exec] == [exec] [exec] [exec] mv: '/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess' and '/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess' are the same file BUILD SUCCESSFUL Total time: 25 minutes 14 seconds Archiving artifacts Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Recording test results Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 [description-setter] Description set: ZOOKEEPER-3142 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Email was triggered for: Success Sending email for trigger: Success Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 ### ## FAILED TESTS (if any) ## All tests passed
ZooKeeper-trunk-windows-cmake - Build # 2940 - Still Failing
See https://builds.apache.org/job/ZooKeeper-trunk-windows-cmake/2940/ ### ## LAST 60 LINES OF THE CONSOLE ### [...truncated 9.76 KB...] [ivy:retrieve] confs: [javacc] [ivy:retrieve] found net.java.dev.javacc#javacc;5.0 in maven2 [ivy:retrieve] :: resolution report :: resolve 25ms :: artifacts dl 1ms - | |modules|| artifacts | | conf | number| search|dwnlded|evicted|| number|dwnlded| - | javacc | 1 | 0 | 0 | 0 || 1 | 0 | - [ivy:retrieve] :: retrieving :: org.apache.zookeeper#zookeeper [ivy:retrieve] confs: [javacc] [ivy:retrieve] 1 artifacts copied, 0 already retrieved (291kB/5ms) generate_jute_parser: [mkdir] Created dir: f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\build\jute_compiler\org\apache\jute\compiler\generated [ivy:artifactproperty] DEPRECATED: 'ivy.conf.file' is deprecated, use 'ivy.settings.file' instead [ivy:artifactproperty] :: loading settings :: file = f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\ivysettings.xml [move] Moving 1 file to f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\build\javacc\lib [javacc] Java Compiler Compiler Version 5.0 (Parser Generator) [javacc] (type "javacc" with no arguments for help) [javacc] Reading from file f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\zookeeper-jute\src\main\java\org\apache\jute\compiler\generated\rcc.jj . . . [javacc] File "TokenMgrError.java" does not exist. Will create one. [javacc] File "ParseException.java" does not exist. Will create one. [javacc] File "Token.java" does not exist. Will create one. [javacc] File "SimpleCharStream.java" does not exist. Will create one. [javacc] Parser generated successfully. jute: [javac] Compiling 39 source files to f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\build\classes [javac] warning: [options] bootstrap class path not set in conjunction with -source 1.8 [javac] 1 warning compile_jute_uptodate: compile_jute: [mkdir] Created dir: f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\src\java\generated [mkdir] Created dir: f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\zookeeper-client\zookeeper-client-c\generated [java] ../../../zookeeper-jute/src/main/resources/zookeeper.jute Parsed Successfully [java] ../../../zookeeper-jute/src/main/resources/zookeeper.jute Parsed Successfully [touch] Creating f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake\src\java\generated\.generated BUILD SUCCESSFUL Total time: 7 seconds [ZooKeeper-trunk-windows-cmake] $ cmd /c call F:\jenkins\jenkins-slave\temp\jenkins2929457438132221234.bat f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake>cd src/c The system cannot find the path specified. f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake>cmake . CMake Error: The source directory "F:/jenkins/jenkins-slave/workspace/ZooKeeper-trunk-windows-cmake" does not appear to contain CMakeLists.txt. Specify --help for usage, or press the help button on the CMake GUI. f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake>cmake --build . Error: could not load cache f:\jenkins\jenkins-slave\workspace\ZooKeeper-trunk-windows-cmake>exit 1 Build step 'Execute Windows batch command' marked build as failure Email was triggered for: Failure - Any Sending email for trigger: Failure - Any ### ## FAILED TESTS (if any) ## No tests ran.
Success: ZOOKEEPER- PreCommit Build #2192
Build: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2192/ ### ## LAST 60 LINES OF THE CONSOLE ### [...truncated 78.57 MB...] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 10 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 core tests. The patch passed core unit tests. [exec] [exec] +1 contrib tests. The patch passed contrib unit tests. [exec] [exec] Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2192//testReport/ [exec] Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2192//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html [exec] Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2192//console [exec] [exec] This message is automatically generated. [exec] [exec] [exec] == [exec] == [exec] Adding comment to Jira. [exec] == [exec] == [exec] [exec] [exec] Comment with id 16617930 added to ZOOKEEPER-1260. [exec] Session logged out. Session was JSESSIONID=34868549A6E5A71256D0C6A02FEF88E5. [exec] [exec] [exec] == [exec] == [exec] Finished build. [exec] == [exec] == [exec] [exec] [exec] mv: '/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess' and '/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/patchprocess' are the same file BUILD SUCCESSFUL Total time: 22 minutes 55 seconds Archiving artifacts Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Recording test results Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 [description-setter] Description set: ZOOKEEPER-1260 Putting comment on the pull request Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Email was triggered for: Success Sending email for trigger: Success Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 Setting JDK_1_8_LATEST__HOME=/home/jenkins/tools/java/latest1.8 ### ## FAILED TESTS (if any) ## All tests passed
[jira] [Commented] (ZOOKEEPER-1260) Audit logging in ZooKeeper servers.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16617930#comment-16617930 ] Hadoop QA commented on ZOOKEEPER-1260: -- +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 10 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2192//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2192//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2192//console This message is automatically generated. > Audit logging in ZooKeeper servers. > --- > > Key: ZOOKEEPER-1260 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1260 > Project: ZooKeeper > Issue Type: New Feature > Components: server >Reporter: Mahadev konar >Assignee: Mohammad Arshad >Priority: Major > Labels: pull-request-available > Fix For: 3.6.0, 3.5.5 > > Attachments: ZOOKEEPER-1260-01.patch, zookeeperAuditLogs.pdf > > Time Spent: 2h > Remaining Estimate: 0h > > Lots of users have had questions on debugging which client changed what znode > and what updates went through a znode. We should add audit logging as in > Hadoop (look at Namenode Audit logging) to log which client changed what in > the zookeeper servers. This could just be a log4j audit logger. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/338 @pmoust I was pointing to comments: [1](https://issues.apache.org/jira/browse/ZOOKEEPER-1260?focusedCommentId=16612302&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16612302), [2](https://issues.apache.org/jira/browse/ZOOKEEPER-1260?focusedCommentId=16612315&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16612315) and [3](https://issues.apache.org/jira/browse/ZOOKEEPER-1260?focusedCommentId=16612318&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16612318) ---
[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...
Github user ivmaykov commented on the issue: https://github.com/apache/zookeeper/pull/184 @anmolnar I wonder if it would be better to remove port unification / unified server socket from #184 in that case, and commit it separately later? It's very easy to bring down a cluster, we had it happen 3 times in 24 hours on a test cluster that had port unification on. Also good points about splitting #627 into several PRs and creating separate JIRAs for them. I'll see what I can do ... ---
[GitHub] zookeeper issue #605: [ZOOKEEPER-3125] Fixing pzxid consistent issue when re...
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/605 @anmolnar, sorry, just saw your comment, I'll send out a PR for 3.5 today. ---
[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.
Github user pmoust commented on the issue: https://github.com/apache/zookeeper/pull/338 @arshadmohammad hm - they don't seem to be any(?) ---
[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.
Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/338 Hi all, please find performance impact analysis in jira comments. ---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r218073947
--- Diff: src/java/main/org/apache/zookeeper/audit/AuditConstants.java ---
@@ -0,0 +1,37 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.audit;
+
+public class AuditConstants {
+public static final String SUCCESS = "success";
+public static final String FAILURE = "failure";
+// operation is performed, result is not known yet
+public static final String INVOKED = "invoked";
+public static final String KEY_VAL_SEPARATOR = "=";
+public static final char PAIR_SEPARATOR = '\t';
+
+public static final String OP_START = "serverStart";
--- End diff --
Operation names are used multiple places through constants. So keeping the
AuditConstants as it is. Also not exposing operation name through ZooDefs
constants.
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r218068308
--- Diff: conf/log4j.properties ---
@@ -63,3 +63,20 @@
log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
### Notice we are including log4j's NDC here (%x)
log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601}
[myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
+#
+# zk audit logging
+#
+zookeeper.auditlog.file=zookeeper_audit.log
+zookeeper.auditlog.threshold=INFO
+audit.logger=INFO, AUDITFILE
+log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
+log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
+log4j.appender.AUDITFILE=org.apache.log4j.RollingFileAppender
--- End diff --
rolling of the audit log file is defined by following properties
log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
# Max log file size of 10MB
log4j.appender.RFAAUDIT.MaxFileSize=10MB
log4j.appender.RFAAUDIT.MaxBackupIndex=10
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r218067686
--- Diff: conf/log4j.properties ---
@@ -63,3 +63,20 @@
log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
### Notice we are including log4j's NDC here (%x)
log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601}
[myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
+#
+# zk audit logging
+#
+zookeeper.auditlog.file=zookeeper_audit.log
+zookeeper.auditlog.threshold=INFO
+audit.logger=INFO, AUDITFILE
+log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
+log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
--- End diff --
log4j.additivity.org.apache.zookeeper.audit = false is superset
configuration of
log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
As all the all audit logging is done through ZKAuditLogger it is ok to
configure only this
---
[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...
Github user arshadmohammad commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/338#discussion_r218066854
--- Diff: src/java/main/org/apache/zookeeper/ZKUtil.java ---
@@ -168,4 +169,41 @@ private static void visitSubTreeDFSHelper(ZooKeeper
zk, final String path,
return; // ignore
}
}
+
+/**
+ * @param perms
+ *ACL permissions
+ * @return string representation of permissions
+ */
+public static String getPermString(int perms) {
--- End diff --
Good suggestion
selecting dynamic caching
---
[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/184 ...and yes, upgrading existing clusters to SSL will not be supported until we fully support port unification. ---
[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/184 Additionally I've noticed that you haven't created a separate Jira for #621 which makes the administration of these issues quite cumbersome. I think the following would be feasible and might be acceptable for everybody: - Get this patch in and leave `portUnification=false` by default. It might be good to add a note in the docs that port unification is experimental and we have a jira to fix it, - Create separate jira for fixing port unification, - Create another jira if needed for other improvements and fixes. - Release 3.5 whatever gets in, but don't mark the new tickets as blockers. I think it's fine if there're known issues in the stable release as long as they don't impact the core functionality and turned off by default. For instance, we have a ticket for TTL nodes as well, but that's not a blocker either. Thoughts? ---
[GitHub] zookeeper issue #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast protoc...
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/184 @ivmaykov Very good point, I cannot tell you unfortunately. I would ask more senior ZooKeeper committers for some background @phunt @hanm and what the best practice would be. I don't see any obstacles why we wouldn't way with 3.5 release for #627 too, but let's see what others are saying. ---
[GitHub] zookeeper pull request #622: [ZOOKEEPER-3145] Fix potential watch missing is...
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/622#discussion_r218055822
--- Diff:
src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java ---
@@ -1861,6 +1862,76 @@ public void testFaultyMetricsProviderOnConfigure()
throws Exception {
Assert.assertTrue("complains about metrics provider
MetricsProviderLifeCycleException", found);
}
+/**
+ * Test leader/leader compatibility with/without CloseSessionTxn, so
that
+ * we can gradually rollout this code and rollback if there is problem.
+ */
+@Test
+public void testCloseSessionTxnCompatile() throws Exception {
--- End diff --
Testing multiple things in a single unit test is not a good practice
either. Also you could add more context to the method names like
`testCloseSessionTxnCompatile_LeaderDisabled_FollowerEnabled()`.
Why not refactoring out to a separate test file then? This file is already
quite overloaded: ~2000 lines of code.
From my experiences waiting for a refactoring ticket to fix these kind of
issues doesn't lead to the resolution. Rectoring smaller chunks of code in PRs
like this is essentially making progress. Following the existing bad practice
is just increasing tech debt.
---
