Testtest #6
Open
Testtest #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![cs-rok-onprem-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53562?s=60&v=4){kind=small}
There was a problem hiding this comment.
Due to limitations of GitHub API we couldn't post all 262 comments in this review.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
Use mavanagaiata plugin to get git commit id for VerGen instead of properties-maven-plugin (which is broken in some environments). Also add the commit to the jar manifests for easy reference when given a jar of unknown origin (especially useful for SNAPSHOT builds). Author: Christopher Tubbs <ctubbsii@apache.org> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1268 from ctubbsii/ZK-3738
…ork is broken - add unit test to verify the bug - bypass the SendThread.startConnect() by throw RuntimeExcepth if state.isAlive is false Author: Fangxi Yin <yinfangxikuaishou.com> Author: yinfangxi <yinfangxi@kuaishou.com> Reviewers: Michael Han <hanm@apache.org>, Enrico Olivelli <eolivelli@gmail.com>, maoling Closes apache#1235 from yfxhust/ZOOKEEPER-3706
This test is always passing for me when I execute only the PurgeTxnTest test class locally, but when I execute all the tests (with multiple parallel threads, using `mvn clean install`), then it always fails. It is failing frequently on the `zookeeper-master-maven` Jenkins job as well. The test starts three threads, performing 1000 ZNode creation in each thread and timeouts if the threads are not finished in 90 seconds. Currently it is not easy to tell based on the logs if the timeout happens because the operations are still in progress or if one of the threads terminated due to an unexpected exception. In this patch I: - increased the timeout from 90 to 120 seconds - added an extra logic to actually fail because of the Exception on the threads, if any happen during the execution - I decreased the number of ZNode creations to 750 (from the original 1000) Applying this patch locally fixed my issues, I hope it will be enough to fix the test on Jenkins as well. Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1274 from symat/ZOOKEEPER-3740
Ever since ZOOKEEPER-107 (released in 3.5.0) the servers are sending their addresses during initial connection requests. The receiving server can potentially use these addresses to send back a new connection request if the challenge is won by the receiver. If the server config contains wildcard address (e.g. 0.0.0.0 in case of IPv4) then the first connection request sent by A to B will contain this address. If the ID of A is smaller than the ID of B, then A will lose the challenge and the second connection request sent back by B will never reach A, as B will send the initial message to 0.0.0.0. So in any 3.5+ ZooKeeper, if wildcard addresses are used in the configs, then there might be some servers never able to rejoin to the quorum after they got restarted. In 3.5+ for backward compatibility reasons (needed during rolling upgrade) there is a version of the QuorumCnxManager.connectOne() method that needs no election address but use the last known address to initiate the connection. In this commit, we simply call this method if the address is a wildcard address. I also added a few restart realted tests, to make sure that restart still works when we don't use wildcard addresses. We can not test the original error with unit tests, as it would require to start the quorum on multiple hosts. I also tested the patch for rolling restart manually both with and without wildcard addresses in the config. Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1254 from symat/ZOOKEEPER-2164
Change-Id: I3c548bcb8e67f83cf6d9fb553d54a6cf9bacf5f3 Author: Patrick Hunt <phunt@apache.org> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Christopher Tubbs, Jan Hentschel Closes apache#1283 from phunt/zk3751
Author: tison <wander4096@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org> Closes apache#1278 from TisonKun/ZOOKEEPER-3745
Since ZooKeeper 3.6.0 we can specify multiple addresses for each ZooKeeper server instance (this can increase availability when multiple physical network interfaces can be used parallel in the cluster). ZooKeeper will perform ICMP ECHO requests or try to establish a TCP connection on port 7 (Echo) of the destination host in order to find the reachable addresses. This should happen only if the user provide multiple addresses in the configuration, in a single address is used then ZooKeeper shouldn’t send any ICMP requests. This works as we expected for the leader election connections, but in this Jira issue we found a bug when the reachability check was performed even with a single address when the Follower tries to connect to the newly elected Leader. The fix follows the same approach we discussed for the election protocol before (see ZOOKEEPER-3698). We avoid the reachability check for single addresses. Also when we have multiple addresses and none of them can be reached, we still start to connect to all addresses in parallel. Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1288 from symat/ZOOKEEPER-3758
Remove unsupported use of com.sun.nio.file.SensitivityWatchEventModifier to better support builds against newer JDKs. Also update build tooling to use strict JDK release compatibility when building with newer JDKs by adding profiles which automatically activate the correct compiler flag when the newer JDK is detected when building with Maven or Eclipse. Author: Christopher Tubbs <ctubbsii@apache.org> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1269 from ctubbsii/ZK-3739
…tibility of applications with 3.5 and 3.6 Author: Enrico Olivelli <enrico.olivelli@diennea.com> Reviewers: Christopher Tubbs, Norbert Kalmar <nkalmar@apache.org>, Mate Szalay-Beko Closes apache#1287 from eolivelli/fix/ZOOKEEPER-3763-compat
Whenever we close the current master ZooKeeper server, a new leader election is triggered. During the new election, a connection will be established between all the servers, by calling the synchronized 'connectOne' method in QuorumCnxManager. The method will open the socket and send a single small initial message to the other server, usually very quickly. If the destination host is unreachable, it should fail immediately. However, when we use Kubernetes, then the destination host is always reachable as it points to Kubernetes services. If the actual container / pod is not available then the 'socket.connect' method will timeout (by default after 5 sec) instead of failing immediately with NoRouteToHostException. As the 'connectOne' method is synchronized, this timeout will block the creation of other connections, so a single unreachable host can cause timeout in the leader election protocol. One workaround is to decrease the socket connection timeout with the '-Dzookeeper.cnxTimeout' stystem property, but the proper fix would be to make the connection initiation fully asynchronous, as using very low timeout can have its own side effect. Fortunately most of the initial message sending is already made async: the SASL authentication can take more time, so the second (authentication + initial message sending) part of the initiation protocol is already called in a separate thread, when Quorum SASL authentication is enabled. In the following patch I made the whole connection initiation async, by always using the async executor (not only when Quorum SASL is enabled) and also moving the socket.connect call into the async thread. I also created a unit test to verify my fix. I added a static socket factory that can be changed by the tests using a packet private setter method. My test failed (and produced the same error logs as we see in the original Jira ticket) before I applied my changes and a time-outed as no leader election succeeded after 15 seconds. After the changes the test runs very quickly, in 1-2 seconds. Note: due to the multiAddress changes, we will need different PRs to the branch 3.5 and to the 3.6+ branches. I will submit the other PR once this got reviewed. Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com> Author: Mate Szalay-Beko <mszalay@cloudera.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1289 from symat/ZOOKEEPER-3756-master
…gured threshold Author: Jie Huang <jiehuang@fb.com> Author: Ivailo Nedelchev <nedelchev@fb.com> Reviewers: Michael Han <hanm@apache.org>, Allan Lyu <fangmin@apache.org>, Damien Diederen <dd@crosstwine.com> Closes apache#1211 from jhuan31/ZOOKEEPER-3683
The `Makefile.am` distributed with the C client defines some per-target `*_CFLAGS` and `*_CXXFLAGS` variables. These however, do not reference `AM_CFLAGS` (resp. AM_CXXFLAGS`, which means that some options (notably `-Wall`) are missing when building subsets of the code. Dixit the [Automake docs](https://www.gnu.org/software/automake/manual/html_node/Program-and-Library-Variables.html): > In compilations with per-target flags, the ordinary ‘AM_’ form of > the flags variable is _not_ automatically included in the > compilation (however, the user form of the variable _is_ included). > So for instance, if you want the hypothetical ‘maude’ compilations > to also use the value of ‘AM_CFLAGS’, you would need to write: > > maude_CFLAGS = ... your flags ... $(AM_CFLAGS) Restoring the flags, however, causes compilation failures (in the library) and a slew of new warnings (in the tests) which had not been noticed because of the missing options. This series of patches (all "tagged" ZOOKEEPER-3654) fix these warnings and errors before re-enabling `-Wall` and friends for all targets. Author: Damien Diederen <dd@crosstwine.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Andor Molnar <andor@apache.org> Closes apache#1186 from ztzg/ZOOKEEPER-3654-incorrect-automake-flags
https://issues.apache.org/jira/browse/ZOOKEEPER-3760 When I upgrade zookeeper from 3.4.13 to 3.5.7 in my application, I find the function processCmd in ZooKeeperMain.java throws a CliException which has been caught in the function. So I think it can be removed Author: lingjinjiang <lingjinjiang@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1286 from lingjinjiang/master
…n the FinalRequestProcessor#processRequest Author: Brittany Barnes <60981978+blb93@users.noreply.github.com> Author: Brittany Barnes <brittany.barnes@ibm.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Justin Ling Mao <maoling199210191@sina.com>, Luciano Resende <lresende@apache.org> Closes apache#1271 from blb93/ZOOKEEPER-3728
Straightforward. The question might be at which level we'd like to maintain jute document. I'm not quite familiar with this section and suspect it is far more outdated. Author: tison <wander4096@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org>, Christopher Tubbs Closes apache#1291 from TisonKun/ZOOKEEPER-3767
…LS config The current zkCli uses system properties to set confidential information like keystore location, password etc. This is not secure as these properties need to be passed on the command line as "-D" arguments. Currently, there is no way to create a ZookeeperAdmin does not have a constructor which takes both canBeReadOnly and ZKClientConfig as parameters. I am introducing a new constructor in ZookeeperAdmin which takes an additional ZKClientConfig parameter. This ZKClientConfig is created by an optional command line argument ``client-configuration``. If no argument is passed, a ZookeeperAdmin object with null client config is created, just like before. Author: Sankalp <sankal@amazon.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org>, Justin Ling Mao <maoling199210191@sina.com> Closes apache#1285 from sankalpbhatia/ZOOKEEPER-3689
- replace ant build.xml with maven pom.xml for zookeeper-contrib-fatjar module - create maven profile "fatjar" to build a fatjar file. - update readme files Author: Sushant Mane <sushant.mane@sjsu.edu> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Benjamin Reed <breed@apache.org> Closes apache#1284 from sushantmane/master
eolivelli generally I use `2to3` util and check the codepath that I can arrive, manually fix some lines. But it seems we can verify this patch totally when merging this patch :) Author: tison <wander4096@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1295 from TisonKun/patch-1
Using a factory design pattern to refactor ZooKeeperMain, making the code more elegant. Author: Jono <jono.morris@xtra.co.nz> Reviewers: andor@apache.org Closes apache#1255 from jono-morris/ZOOKEEPER-3581 and squashes the following commits: 556e176 [Jono] Fix style errors with import statements. e10dfb8 [Jono] Add licence text. 4957d89 [Jono] 1. Reinstate missing imports to ZooKeeperMain. ed76f14 [Jono] Minor Javadoc updates. cc8de49 [Jono] Add Factory to create CliCommand classes. Initial Commit.
Using ZooKeeper with JDK 12.0.2 on CentOS 7 when the current leader is killed, we saw a few times that some partial Leader Election notification (vote) messages were delivered to the other ZooKeeper servers. The malformed / partial messages are causing different exceptions in the WorkerReceiver thread of FastLeaderElection which were not handled before. This was leading to the death of the WorkerReceiver thread, which caused that the given ZooKeeper Server was unable to receive leader election messages anymore and was not able to re-join to any quorum until it got restarted. In the proposed fix I created unit tests to simulate certain error cases with regards to partial leader election messages, and fixed the error handling in FastLeaderElection. Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes apache#1300 from symat/ZOOKEEPER-3769-master
Added a warning in https://cwiki.apache.org/confluence/display/ZOOKEEPER/Upgrade+FAQ Author: Norbert Kalmar <nkalmar@apache.org> Reviewers: Enrico Olivelli <eolivelli@apache.org> Closes apache#1304 from nkalmar/branch-3.6 (cherry picked from commit 5062c39) Signed-off-by: Enrico Olivelli <eolivelli@apache.org>
Author: Enrico Olivelli <eolivelli@apache.org> Reviewers: Mate Szalay-Beko <szalay.beko.mate@gmail.com>, TisunKun <wander4096@gmail.com> Closes apache#1308 from eolivelli/fix/jdk14
…g list
After Py3, `filter` return a `filter object` instead of `list object`, which causes
```
Traceback (most recent call last):
File "zk-merge-pr.py", line 533, in <module>
main()
File "zk-merge-pr.py", line 519, in main
resolve_jira_issues(commit_title, merged_refs, jira_comment)
File "zk-merge-pr.py", line 329, in resolve_jira_issues
resolve_jira_issue(merge_branches, comment, jira_id)
File "zk-merge-pr.py", line 312, in resolve_jira_issue
jira_fix_versions = [get_version_json(v) for v in fix_versions]
File "zk-merge-pr.py", line 312, in <listcomp>
jira_fix_versions = [get_version_json(v) for v in fix_versions]
File "zk-merge-pr.py", line 310, in get_version_json
return filter(lambda v: v.name == version_str, versions)[0].raw
TypeError: 'filter' object is not subscriptable
```
We can replace filter with list comprehension to fix it.
Author: tison <wander4096@gmail.com>
Reviewers: Enrico Olivelli <eolivelli@apache.org>
Closes apache#1303 from TisonKun/ZOOKEEPER-3782
…rl client
This patch allows one to access the C client Cyrus SASL support (ZOOKEEPER-1112) from the Perl binding by passing a `--with-sasl2` flag (and, optionally, header and lib locations):
perl Makefile.PL \
--with-sasl2 \
--sasl2-include=/path/to/sasl2/include \
--sasl2-lib=/path/to/sasl2/lib
When enabled, `Net::ZooKeeper->new(...)` admits a new key, `sasl_options`, which can be used to automatically authenticate with the server during connections (including reconnects).
Some of the preparatory work for this contribution, which is available in the other commits, may also fix [ZOOKEEPER-3303](https://issues.apache.org/jira/browse/ZOOKEEPER-3303), which is about compilation issues. The resulting patch builds with GCC 8.3.0 in a (moderately) "hardened" environment; it also passes all enabled tests.
Author: Damien Diederen <dd@crosstwine.com>
Reviewers: Norbert Kalmar <nkalmar@apache.org>
Closes apache#1243 from ztzg/ZOOKEEPER-3714-zkperl-sasl-support
Simplify generation of VersionInfoMain.java and Info.java by using maven-resource-plugin's built-in resource filtering at build time. This eliminates the need to use VerGen to generate java source files during the build. Also make other slight pom improvements: 1. Remove trailing tab character in Ted's name in pom.xml 2. Simplify spotbugs skipping in contrib pom.xml 3. Add m2e configuration for build plugin executions to be ignored by Eclipse, for developers (like me) using Eclipse IDE 4. Format build time in a more international-friendly and less ambiguous way (year first, then month, then day, using UTC instead of GMT) Link to issue: https://issues.apache.org/jira/browse/ZOOKEEPER-3786 Author: Christopher Tubbs <ctubbsii@apache.org> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1310 from ctubbsii/use-resource-filtering-for-version-info
Fix for https://issues.apache.org/jira/browse/ZOOKEEPER-3726 sockaddr_storage can contain ipv4 or ipv6 address. If address is ipv6, then we need to compare more bytes. In this PR correct comparison of sockaddr_storage was added. Author: Vladislav Tyulbashev <vtyulb@vtyulb.ru> Reviewers: Norbert Kalmar <nkalmar@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1252 from vtyulb/ZOOKEEPER-3726
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
There was a problem hiding this comment.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
| @@ -271,7 +271,7 @@ public void processResult(int rc, String path, Object ctx, List<String> children | |||
| } | |||
There was a problem hiding this comment.
❌ New issue: Complex Method
processResult has a cyclomatic complexity of 16, threshold = 9
| @@ -271,7 +271,7 @@ public void processResult(int rc, String path, Object ctx, List<String> children | |||
| } | |||
There was a problem hiding this comment.
❌ New issue: Bumpy Road Ahead
processResult has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function
| @@ -271,7 +271,7 @@ public void processResult(int rc, String path, Object ctx, List<String> children | |||
| } | |||
There was a problem hiding this comment.
❌ New issue: Overall Code Complexity
This module has a mean cyclomatic complexity of 4,14 across 14 functions. The mean complexity threshold is 4
| @@ -271,7 +271,7 @@ public void processResult(int rc, String path, Object ctx, List<String> children | |||
| } | |||
There was a problem hiding this comment.
❌ New issue: Deep, Nested Complexity
processResult has a nested complexity depth of 4, threshold = 4
| @@ -271,7 +271,7 @@ public void processResult(int rc, String path, Object ctx, List<String> children | |||
| } | |||
There was a problem hiding this comment.
❌ New issue: Primitive Obsession
In this module, 70,4% of all function arguments are primitive types, threshold = 30,0%
| @@ -86,7 +86,7 @@ public class GenerateLoad { | |||
| synchronized static void add(long time, int count, Socket s) { | |||
There was a problem hiding this comment.
❌ New issue: Complex Method
GenerateLoad.GeneratorInstance.ZooKeeperThread.run has a cyclomatic complexity of 9, threshold = 9
| @@ -86,7 +86,7 @@ public class GenerateLoad { | |||
| synchronized static void add(long time, int count, Socket s) { | |||
There was a problem hiding this comment.
❌ New issue: Bumpy Road Ahead
main has 4 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function
| @@ -86,7 +86,7 @@ public class GenerateLoad { | |||
| synchronized static void add(long time, int count, Socket s) { | |||
There was a problem hiding this comment.
❌ New issue: Deep, Nested Complexity
main has a nested complexity depth of 5, threshold = 4
| @@ -86,7 +86,7 @@ public class GenerateLoad { | |||
| synchronized static void add(long time, int count, Socket s) { | |||
There was a problem hiding this comment.
❌ New issue: Primitive Obsession
In this module, 68,0% of all function arguments are primitive types, threshold = 30,0%
| @@ -86,7 +86,7 @@ public class GenerateLoad { | |||
| synchronized static void add(long time, int count, Socket s) { | |||
There was a problem hiding this comment.
❌ New issue: Excess Number of Function Arguments
GenerateLoad.GeneratorInstance.ZooKeeperThread.processResult has 5 arguments, threshold = 4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
dd