-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Branch 3.7.0 #2
base: master
Are you sure you want to change the base?
Branch 3.7.0 #2
Conversation
1.update checkstyle to latest version 8.39 See [https://checkstyle.sourceforge.io/config_javadoc.html](https://checkstyle.sourceforge.io/config_javadoc.html) - JavadocMethod: remove properties `allowMissingJavadoc,allowMissingThrowsTags,allowThrowsTagsForSubclasses,allowUndeclaredRTE` - LineLength: change it's parent to Checker 2.update XML dtd: `checkstyle-strict.xml, checkstyle-simple.xml, checkstyleSuppressions.xml` 3.fix code style: - `QuorumPeer.java, PemReader.java`. Operators like + and ? appear at newlines rather than at the end of the previous line. - license of `TestApacheCuratorCompatibility.java`. Checkstyle JavadocParagraph:`<p>` tag should be placed immediately before the first word. Author: lan <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Damien Diederen <[email protected]> Closes apache#1579 from lanicc/ZOOKEEPER-4058 (cherry picked from commit e8dc2b3) Signed-off-by: Damien Diederen <[email protected]>
- Thanks for the original work from ZOOKEEPER-1383, ZOOKEEPER-2593, ZOOKEEPER-451, especially the work from ZOOKEEPER-1383 contributed by [Thawan Kooburat](https://issues.apache.org/jira/secure/ViewProfile.jspa?name=thawan)(I also sign off his name in the commit message) which also implemented the very valuable throughput quota.In the further, we will also do this. - `zookeeper.enforeQuota`. When enabled and the client exceeds the total bytes or children count hard quota under a znode, the server will reject the request and reply the client a `QuotaExceededException` by force. The default value is: false. - the checkQuota involves the `create()` and `setData()` api, not including the `delete()`. - When users set the quota which's less than the existing stats, we give a thoughtful warning info. - the following code in the StatsTrack has a bad augmentability: > if (split.length != 2) { > throw new IllegalArgumentException("invalid string " + stats); > } we do a trick to solve it for the expansibility, but we will get a little strange quota info(`Output quota for /c2 count=-1,bytes=-1=;byteHardLimit=-1;countHardLimit=5`) when using `listquota`. some UTs has covered it. - the logic about `checkQuota` should be put in the `PrepRequestProcessor`, other than `DataTree`. we will get the following two negative effects if putting `checkQuota` in the `DataTree`: - 1. When the write request has exceeded the quota, the corresponding transaction log will load into disk successfully.It's not good, although it has any data inconsistency issue, because when the server restart, so long as the transaction logs are applied in the same order, the exceeded nodes will not be applied into the state machine. - 2. the client will be blocking and waiting for the response, because when throwing `QuotaExceededException` in the the `DataTree`, the` rc.stat` will be `null` and `BinaryOutputArchive#writeRecord` will throw `NPE`. - 3. Overall, the pre-check about the write request should be done in the `PrepRequestProcessor`(at least before `SyncRequestProcessor`)(Look at an example from `checkACL()`) - more detail in the [ZOOKEEPER-3301](https://issues.apache.org/jira/browse/ZOOKEEPER-3301). - [Added in 2020-02-25] use `RateLogger` to replace `LOG` to avoid quota exceed logs flooding the disk - A `TODO` improvement is: only users have admin permission can write to `/zookeeper/quota`(just like `/zookeeper/config`) to avoid some users' misoperation Author: maoling <[email protected]> Reviewers: Mate Szalay-Beko <[email protected]>, Damien Diederen <[email protected]>, Enrico Olivelli <[email protected]>, Michael Han <[email protected]> Closes apache#934 from maoling/ZOOKEEPER-3301 (cherry picked from commit 190a227) Signed-off-by: Damien Diederen <[email protected]>
… tarball symat noticed that the source tarball for 3.7.0rc0 is missing executable bits. ztzg noticed that this can be worked around by reinstating the "old" version of the maven-assembly-plugin, which had been aligned in ZOOKEEPER-3833. This patch implements the work around, and also applies cleanly on top of `branch-3.7.0` and `branch-3.7`. Also discussed on dev: https://mail-archives.apache.org/mod_mbox/zookeeper-dev/202101.mbox/%3C875z3n9w75.fsf%40crosstwine.com%3E Original report: https://mail-archives.apache.org/mod_mbox/zookeeper-dev/202101.mbox/%3cCAAMoRKLMf7tLosgqyiwYfFxXq-Zmiz=0oTGDijX5M=MHDF_JCgmail.gmail.com%3e Author: Damien Diederen <[email protected]> Reviewers: Enrico Olivelli <[email protected]> Closes apache#1586 from ztzg/ZOOKEEPER-4191-missing-x-bits-source-tarball (cherry picked from commit e7d67da) Signed-off-by: Damien Diederen <[email protected]>
…d not displaying properly ISSUE --- See https://issues.apache.org/jira/browse/ZOOKEEPER-3943 for details on the issue. There are two main things being addressed in this PR: 1. The Maven build for ZooInspector seems to generate an invalid JAR file that is missing graphical resources that cause the application to be non-functional (UI does not render properly, NullPointerExceptions occur constantly, etc.). 2. The current Maven build instructions and run scripts for ZooInspector involve building, moving JAR files around, using relative entries on the CLASSPATH and running the scripts from the correct directory in order to use the `zooInspector.sh`/`zooInspector.cmd` launch scripts. FIXES --- For number 1, the fix is to add the `src/main/resources/*` directories to the JAR artifact built by Maven. For number 2, I've proposed changing the Maven build to use the [Maven Assembly Plugin](http://maven.apache.org/plugins/maven-assembly-plugin/) to build a single "fat jar" for ZooInspector that contains all of its dependencies. The result is that building and running the tool is easier and more straightforward (in my opinion): ``` git clone https://github.com/apache/zookeeper.git cd zookeeper mvn install -DskipTests cd zookeeper-contrib/zookeeper-contrib-zooinspector mvn install ./zooInspector.sh ``` And based on the "fat jar" style of build and updates to the run scripts, the `zooInspector.sh` and `zooInspector.cmd` commands now successfully execute from any directory (once the project is built) as opposed to requiring you to be in the same directory as the scripts. My hope is that these changes allow people like myself who are mainly interested in using ZooInspector to easily clone the repository, build and run the tool without any issues. I spent a chunk of time getting this all working for myself and I hope I can save others some trouble. TESTING --- I've tested cloning, building and running ZooInspector on Windows 10, Mac OS X Mojave (10.14.6) and Ubuntu Linux 18.04 using the steps above (on Java 8). I ran `mvn verify spotbugs:check checkstyle:check -Pfull-build -Dsurefire-forkcount=4` in the root directory (per https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute#HowToContribute-FinalChecksonPullRequest) and got these results: ``` [INFO] [ERROR] Tests run: 2881, Failures: 4, Errors: 4, Skipped: 4 [INFO] [INFO] ------------------------------------------------------------------------ [INFO] Reactor Summary for Apache ZooKeeper 3.7.0-SNAPSHOT: [INFO] [INFO] Apache ZooKeeper ................................... SUCCESS [ 7.731 s] [INFO] Apache ZooKeeper - Documentation ................... SUCCESS [ 3.223 s] [INFO] Apache ZooKeeper - Jute ............................ SUCCESS [ 32.705 s] [INFO] Apache ZooKeeper - Server .......................... FAILURE [30:28 min] [INFO] Apache ZooKeeper - Metrics Providers ............... SKIPPED [INFO] Apache ZooKeeper - Prometheus.io Metrics Provider .. SKIPPED [INFO] Apache ZooKeeper - Client .......................... SKIPPED [INFO] Apache ZooKeeper - Client - C ...................... SKIPPED [INFO] Apache ZooKeeper - Recipes ......................... SKIPPED [INFO] Apache ZooKeeper - Recipes - Election .............. SKIPPED [INFO] Apache ZooKeeper - Recipes - Lock .................. SKIPPED [INFO] Apache ZooKeeper - Recipes - Queue ................. SKIPPED [INFO] Apache ZooKeeper - Assembly ........................ SKIPPED [INFO] Apache ZooKeeper - Compatibility Tests ............. SKIPPED [INFO] Apache ZooKeeper - Compatibility Tests - Curator ... SKIPPED [INFO] Apache ZooKeeper - Tests ........................... SKIPPED [INFO] Apache ZooKeeper - Contrib ......................... SKIPPED [INFO] Apache ZooKeeper - Contrib - Fatjar ................ SKIPPED [INFO] Apache ZooKeeper - Contrib - Loggraph .............. SKIPPED [INFO] Apache ZooKeeper - Contrib - Rest .................. SKIPPED [INFO] Apache ZooKeeper - Contrib - ZooInspector .......... SKIPPED [INFO] ------------------------------------------------------------------------ [INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 31:12 min [INFO] Finished at: 2020-12-05T01:08:27Z [INFO] ------------------------------------------------------------------------ ``` Since all of my proposed changes are in the `zookeeper-contrib` subtree, I'm assuming this doesn't have anything to do with my changes. Running the same command in the `zookeeper-contrib` directory seems to pass for those tests: ``` [INFO] ------------------------------------------------------------------------ [INFO] Reactor Summary for Apache ZooKeeper - Contrib 3.7.0-SNAPSHOT: [INFO] [INFO] Apache ZooKeeper - Contrib ......................... SUCCESS [ 8.590 s] [INFO] Apache ZooKeeper - Contrib - Fatjar ................ SUCCESS [ 13.636 s] [INFO] Apache ZooKeeper - Contrib - Loggraph .............. SUCCESS [ 20.515 s] [INFO] Apache ZooKeeper - Contrib - Rest .................. SUCCESS [ 13.394 s] [INFO] Apache ZooKeeper - Contrib - ZooInspector .......... SUCCESS [ 17.056 s] [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 01:14 min [INFO] Finished at: 2020-12-05T01:13:22Z [INFO] ------------------------------------------------------------------------ ``` Author: brentwritescode <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Damien Diederen <[email protected]> Closes apache#1551 from brentwritescode/master (cherry picked from commit 62e29cc) Signed-off-by: Damien Diederen <[email protected]>
Author: maoling <[email protected]> Reviewers: Damien Diederen <[email protected]> Closes apache#1585 from maoling/ZOOKEEPER-4188 (cherry picked from commit a4d7586) Signed-off-by: Damien Diederen <[email protected]>
This reverts commit 519d24c.
This reverts commit 84d982d.
…wers is empty" when not specifically run from the zookeeper-contrib/zookeeper-contrib-zooinspector directory ISSUE --- See https://issues.apache.org/jira/browse/ZOOKEEPER-4050 for details on the issue. This is a follow-on PR to issues identified in apache#1551. While that PR fixed some launch issues, currently ZooInspector still needs to be run from the root ZooInspector directory because it expects the `defaultConnectionSettings.cfg` and `defaultNodeViewers.cfg` to exist on the filesystem in a specific location. The previous PR ensured that these files are now bundled into the fat jar built by Maven, so this new PR makes the checks for these files fall back to checking the classpath (i.e. checking inside the jar) for these files if they can't be found on the filesystem first. This means that the `zooInspector.sh` and `zooInspector.cmd` scripts can now be run from anywhere once the project is built. TESTING --- I've tested cloning, building and running ZooInspector on Mac OS Catalina (10.15.7) on Java 8 with these fixes and invoking `zooInspector.sh` from different directories to ensure it runs properly and doesn't display the aforementioned error. I ran `mvn verify spotbugs:check checkstyle:check -Pfull-build -Dsurefire-forkcount=4` in the `zookeeper-contrib/zookeeper-contrib-zooinspector` directory (per https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute#HowToContribute-FinalChecksonPullRequest) and got these results: ``` [INFO] You have 0 Checkstyle violations. [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 15.037 s [INFO] Finished at: 2021-02-01T20:30:04-08:00 [INFO] ------------------------------------------------------------------------ ``` Since all of my proposed changes are in the `zookeeper-contrib` subtree (and specifically only in `zookeeper-contrib-zooinspector`, I did not run the wider unit tests for the Zookeeper project as a whole. Author: brentwritescode <[email protected]> Author: Brent Nash <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Damien Diederen <[email protected]> Closes apache#1589 from brentwritescode/ZOOKEEPER-4050 (cherry picked from commit 245ff75) Signed-off-by: Damien Diederen <[email protected]>
I believe we don't need to add the git checkout to the "Steps" section. I cannot see it neither in owasp nor the PR jenkinsfiles. Also I see that master branch gets also checked out during our normal builds, so I hope this will fix it. Target branches: master, branch-3.7, branch-3.6, branch-3.5 Author: Andor Molnar <[email protected]> Reviewers: Enrico Olivelli <[email protected]> Closes apache#1600 from anmolnar/ZOOKEEPER-4207 (cherry picked from commit 8c68933) Signed-off-by: Enrico Olivelli <[email protected]>
Increase test stability by avoiding test failures due to port collisions by preventing tests from running concurrently in the GitHub Actions CI builds. Author: Christopher Tubbs <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Damien Diederen <[email protected]> Closes apache#1606 from ctubbsii/reduce-forkCount (cherry picked from commit c26f96c) Signed-off-by: Damien Diederen <[email protected]>
Update Netty to 4.1.59.Final on to address the vulnerability described at https://snyk.io/vuln/SNYK-JAVA-IONETTY-1020439 Author: Frederiko Costa <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Damien Diederen <[email protected]> Closes apache#1605 from frederiko/netty-4.1.59-update (cherry picked from commit 884fc38) Signed-off-by: Damien Diederen <[email protected]>
`WatcherCleanerTest` performs latency checks which fail when outside of a 20+Xms window. Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina. This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM. Author: Damien Diederen <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> Closes apache#1592 from ztzg/ZOOKEEPER-4200-widen-latency-window (cherry picked from commit d8ff555) Signed-off-by: Enrico Olivelli <[email protected]>
This patch works around the numerous deprecation notices added to the CyrusSASL library on macOS. It is a direct "port" of the solution to MESOS-3030, which hit exactly the same problem: https://issues.apache.org/jira/browse/MESOS-3030 https://reviews.apache.org/r/39230/diff/3/ The PR also includes a fix for the the `clockid_t` compilation issue mentioned in the ticket description, but the test suite as a whole remains broken on macOS as its linker does not support the `--wrap` option. Author: Damien Diederen <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> Closes apache#1593 from ztzg/ZOOKEEPER-4201-catalina-c-client-fixes (cherry picked from commit 58b4c10) Signed-off-by: Enrico Olivelli <[email protected]>
…ty is true snapshot.trust.empty is an escape hatch for users upgrading from 3.4.x to later Zookeeper versions, allowing nodes to start with a non-empty transaction log but no snapshot. The intent is for this setting to be enabled for a short while during the upgrade, and then disabled again, as the check it disables is a safety feature. Prior to this PR, a node would only write a snapshot locally if it became leader, or if it had fallen so far behind the leader that the leader sent a SNAP message instead of a DIFF. This made the upgrade process inconvenient, as not all nodes would create a snapshot when snapshot.trust.empty was true, meaning that the safety check could not be flipped back on. This PR makes follower nodes write a local snapshot when they receive NEWLEADER, if they have no local snapshot and snapshot.trust.empty is true. Author: Stig Rohde Døssing <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Damien Diederen <[email protected]> Closes apache#1581 from srdo/zookeeper-3781 (cherry picked from commit 1214d3b) Signed-off-by: Damien Diederen <[email protected]>
…e.maxbufer size Author: Mathieu Marie <[email protected]> Reviewers: Damien Diederen <[email protected]>, Mate Szalay-Beko <[email protected]> Closes apache#1614 from mariemat/ZOOKEEPER-4221 (cherry picked from commit 94d0c4d) Signed-off-by: Mate Szalay-Beko <[email protected]>
Without this patch, a multi() transaction such as the one implemented in ZooKeeperQuotaTest.testMultiCreateThenSetDataShouldWork fails with MarshallingError when 'enforceQuota' is enabled. This happens whenever the node has an associated quota, whether it was exceeded or not. This is due to the server encountering null while trying to access a database node by path--whereas that node only exists as a ChangeRecord in the server's 'outstandingChanges' list: java.lang.NullPointerException at org.apache.zookeeper.server.ZooKeeperServer.checkQuota(ZooKeeperServer.java:2048) at org.apache.zookeeper.server.PrepRequestProcessor.pRequest2Txn(PrepRequestProcessor.java:397) The patch adds an additional 'lastData' parameter to the quota checking function, and passes the data from the ChangeRecord during 'setData' operations. Author: Damien Diederen <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>, Norbert Kalmar <[email protected]> Closes apache#1611 from ztzg/ZOOKEEPER-4219-quota-multi-setdata (cherry picked from commit eb1569e) Signed-off-by: Norbert Kalmar <[email protected]>
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`. This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test. Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters. The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up. The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`. Author: Damien Diederen <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> Closes apache#1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
… election We have a logic in the server code, that would try to connect to an other quorum member, based on its server ID. We identify the address assigned to this ID first based on the last committed quorum configuration. If the connection attempt fails (or the server is not known in the committed configuration) then we try to find the address based on the last proposed quorum configuration. But we should do the second connection attempt, only if the address in the last proposed configuration differs from the address in the last committed configuration. Otherwise we would just retry to connect to the same address that failed just right before. In the current code we have a bug, because we compare the address object references (use "!=") instead of comparing the objects themselves (using "not equals"). In certain edge cases (e.g. when the last proposed and last committed addresses are the same, but the address is unreachable) this bug can lead to unnecessary retry of connection attempts. The normal behaviour would be to mark this connection attempt to be failed and wait for e.g. the next election round or wait for the other server to come online and initiate a connection to us. Author: Mate Szalay-Beko <[email protected]> Reviewers: Andor Molnar <[email protected]>, Damien Diederen <[email protected]> Closes apache#1615 from symat/ZOOKEEPER-4220 (cherry picked from commit 6022e03) Signed-off-by: Damien Diederen <[email protected]>
…quare bracket same as LocalPeerBean …quare bracket same as LocalPeerBean Author: Mohammad Arshad <[email protected]> Reviewers: Enrico Olivelli <[email protected]> Closes apache#1493 from arshadmohammad/ZOOKEEPER-3877-master (cherry picked from commit 425ee18) Signed-off-by: Mohammad Arshad <[email protected]>
`InvalidSnapshotTest.testSnapshot` starts an instance of `ZooKeeperServer` on the version-controlled `resources/data/invalidsnap` directory, which, as a side-effect, \"fixes\" the following snapshot—which was broken on purpose (see ZOOKEEPER-367): `zookeeper-server/src/test/resources/data/invalidsnap/version-2/snapshot.83f` This status quo creates a number of problems: 1. It makes the test ineffective after the first run; 2. The file shows as modified in version control tools, which can be annoying; 3. The \"fixed\" snapshot can end up being committed by mistake, invalidating the test. (\#3 is not theoretical; that \"fixed\" snapshot frequently shows up in pull requests, and was recently merged into `master`.) Author: Damien Diederen <[email protected]> Reviewers: Mohammad Arshad <[email protected]> Closes apache#1627 from ztzg/ZOOKEEPER-4232-invalid-snapshot-is-invalid-x-3.7 and squashes the following commits: c18a895 [Damien Diederen] ZOOKEEPER-4232: Ensure that ZOOKEEPER-367 test data fails to parse 42a5a0b [Damien Diederen] ZOOKEEPER-4232: Run InvalidSnapshotTest on a copy of test data (cherry picked from commit 4a19dd1) Signed-off-by: Mohammad Arshad <[email protected]>
…20 - CVE-2020-27223 The OWASP checker reports that the version of Jetty currently referenced by this branch is vulnerable to a CVE: [ERROR] Failed to execute goal org.owasp:dependency-check-maven:5.3.0:check (default-cli) on project zookeeper: [ERROR] [ERROR] One or more dependencies were identified with vulnerabilities that have a CVSS score greater than or equal to '0.0': [ERROR] [ERROR] jetty-server-9.4.35.v20201120.jar: CVE-2020-27223 [ERROR] jetty-http-9.4.35.v20201120.jar: CVE-2020-27223 https://nvd.nist.gov/vuln/detail/CVE-2020-27223 describes it as: > In Eclipse Jetty 9.4.6.v20170531 to 9.4.36.v20210114 (inclusive), > 10.0.0, and 11.0.0 when Jetty handles a request containing multiple > Accept headers with a large number of "quality" (i.e. q) parameters, > the server may enter a denial of service (DoS) state due to high CPU > usage processing those quality values, resulting in minutes of CPU > time exhausted processing those quality values. This changeset bumps Jetty to 9.4.38.v20210224, which is the latest as of the commit date. Author: Damien Diederen <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mohammad Arshad <[email protected]> Closes apache#1623 from ztzg/ZOOKEEPER-4023-jetty-CVE-2020-27223 and squashes the following commits: 7cb65fb [Damien Diederen] zookeeper-server: Distribution tarball does not include jetty-client 59cffa1 [Damien Diederen] ZOOKEEPER-4233: dependency-check:check failing - Jetty 9.4.35.v20201120 - CVE-2020-27223 (cherry picked from commit 2f98b8f) Signed-off-by: Mohammad Arshad <[email protected]>
… in RestMain Author: Mukti <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mohammad Arshad <[email protected]> Closes apache#1633 from MuktiKrishnan/ZOOKEEPER-4230-master (cherry picked from commit 04471b2) Signed-off-by: Mohammad Arshad <[email protected]>
…executing commands -waitforconnection option will make zk client wait for -timeout time to connect to zk server. timeout time is 30ms by default but can be specified explicitly for a session using -timeout option in command line. Author: Mukti <[email protected]> Reviewers: maoling <[email protected]>, Mohammad Arshad <[email protected]> Closes apache#1626 from MuktiKrishnan/ZOOKEEPER-1871-master and squashes the following commits: 2947514 [Mukti] ZOOKEEPER-1871: Removed statement which was re-creating zookeeper admin in ZooKeeperMain.java and added documentation for waitforconnection in zookeeperCLI.md and c475d46 [Mukti] ZOOKEEPER-1871: Add an option to zkCli to wailt for connection before executing commands (cherry picked from commit 2e40011) Signed-off-by: Mohammad Arshad <[email protected]>
Author: Mohammad Arshad <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Damien Diederen <[email protected]> Closes apache#1635 from arshadmohammad/ZOOKEEPER-4227-branch-3.7
This PR adds documentation about ZooKeeper's snapshot compression feature (see [ZOOKEEPER-3179](https://issues.apache.org/jira/browse/ZOOKEEPER-3179)) to `branch-3.7`. Author: Abhilash Kishore <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mohammad Arshad <[email protected]>, Damien Diederen <[email protected]> Closes apache#1642 from abhilash1in/ZOOKEEPER-4231-branch-3.7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CONCLUDE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CONCLUD2E
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CONCLUD2E
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- View detailed results in CodeScene
- Quality Gates: FAILED
- Recommended Review Level: Detailed -- Inspect the code that degrades in code health.
@@ -394,6 +394,7 @@ protected void pRequest2Txn(int type, long zxid, Request request, Record record, | |||
validatePath(path, request.sessionId); | |||
nodeRecord = getRecordForPath(path); | |||
zks.checkACL(request.cnxn, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo, path, null); | |||
zks.checkQuota(path, nodeRecord.data, setDataRequest.getData(), OpCode.setData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pRequest2Txn increases in Lines of Code from 291 to 292
@@ -697,6 +698,7 @@ private void pRequest2TxnCreate(int type, Request request, Record record, boolea | |||
throw new KeeperException.NoChildrenForEphemeralsException(path); | |||
} | |||
int newCversion = parentRecord.stat.getCversion() + 1; | |||
zks.checkQuota(path, null, data, OpCode.create); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pRequest2TxnCreate increases in Lines of Code from 78 to 79
@@ -394,6 +394,7 @@ protected void pRequest2Txn(int type, long zxid, Request request, Record record, | |||
validatePath(path, request.sessionId); | |||
nodeRecord = getRecordForPath(path); | |||
zks.checkACL(request.cnxn, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo, path, null); | |||
zks.checkQuota(path, nodeRecord.data, setDataRequest.getData(), OpCode.setData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pRequest2Txn n-args = 5
} else if (keeperException instanceof KeeperException.QuotaExceededException) { | ||
return "Quota has exceeded : " + keeperException.getPath(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getMessage increase in cyclomatic complexity from 13 to 14
@@ -537,18 +512,18 @@ public void createNode(final String path, byte[] data, List<ACL> acl, long ephem | |||
if (Quotas.limitNode.equals(childName)) { | |||
// this is the limit node | |||
// get the parent and add it to the trie | |||
pTrie.addPath(parentName.substring(quotaZookeeper.length())); | |||
pTrie.addPath(Quotas.trimQuotaPath(parentName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createNode n-args = 8
case QUOTAEXCEEDED: | ||
return "Quota has exceeded"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getCodeMessage increase in cyclomatic complexity from 30 to 31
private void checkQuota(String lastPrefix, long bytesDiff, long countDiff) | ||
throws KeeperException.QuotaExceededException { | ||
LOG.debug("checkQuota: lastPrefix={}, bytesDiff={}, countDiff={}", lastPrefix, bytesDiff, countDiff); | ||
|
||
// now check the quota we set | ||
String limitNode = Quotas.limitPath(lastPrefix); | ||
DataNode node = getZKDatabase().getNode(limitNode); | ||
StatsTrack limitStats; | ||
if (node == null) { | ||
// should not happen | ||
LOG.error("Missing limit node for quota {}", limitNode); | ||
return; | ||
} | ||
synchronized (node) { | ||
limitStats = new StatsTrack(node.data); | ||
} | ||
//check the quota | ||
boolean checkCountQuota = countDiff != 0 && (limitStats.getCount() > -1 || limitStats.getCountHardLimit() > -1); | ||
boolean checkByteQuota = bytesDiff != 0 && (limitStats.getBytes() > -1 || limitStats.getByteHardLimit() > -1); | ||
|
||
if (!checkCountQuota && !checkByteQuota) { | ||
return; | ||
} | ||
|
||
//check the statPath quota | ||
String statNode = Quotas.statPath(lastPrefix); | ||
node = getZKDatabase().getNode(statNode); | ||
|
||
StatsTrack currentStats; | ||
if (node == null) { | ||
// should not happen | ||
LOG.error("Missing node for stat {}", statNode); | ||
return; | ||
} | ||
synchronized (node) { | ||
currentStats = new StatsTrack(node.data); | ||
} | ||
|
||
//check the Count Quota | ||
if (checkCountQuota) { | ||
long newCount = currentStats.getCount() + countDiff; | ||
boolean isCountHardLimit = limitStats.getCountHardLimit() > -1 ? true : false; | ||
long countLimit = isCountHardLimit ? limitStats.getCountHardLimit() : limitStats.getCount(); | ||
|
||
if (newCount > countLimit) { | ||
String msg = "Quota exceeded: " + lastPrefix + " [current count=" + newCount + ", " + (isCountHardLimit ? "hard" : "soft") + "CountLimit=" + countLimit + "]"; | ||
RATE_LOGGER.rateLimitLog(msg); | ||
if (isCountHardLimit) { | ||
throw new KeeperException.QuotaExceededException(lastPrefix); | ||
} | ||
} | ||
} | ||
|
||
//check the Byte Quota | ||
if (checkByteQuota) { | ||
long newBytes = currentStats.getBytes() + bytesDiff; | ||
boolean isByteHardLimit = limitStats.getByteHardLimit() > -1 ? true : false; | ||
long byteLimit = isByteHardLimit ? limitStats.getByteHardLimit() : limitStats.getBytes(); | ||
if (newBytes > byteLimit) { | ||
String msg = "Quota exceeded: " + lastPrefix + " [current bytes=" + newBytes + ", " + (isByteHardLimit ? "hard" : "soft") + "ByteLimit=" + byteLimit + "]"; | ||
RATE_LOGGER.rateLimitLog(msg); | ||
if (isByteHardLimit) { | ||
throw new KeeperException.QuotaExceededException(lastPrefix); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkQuota cyclomatic complexity = 23
private void checkQuota(String lastPrefix, long bytesDiff, long countDiff) | ||
throws KeeperException.QuotaExceededException { | ||
LOG.debug("checkQuota: lastPrefix={}, bytesDiff={}, countDiff={}", lastPrefix, bytesDiff, countDiff); | ||
|
||
// now check the quota we set | ||
String limitNode = Quotas.limitPath(lastPrefix); | ||
DataNode node = getZKDatabase().getNode(limitNode); | ||
StatsTrack limitStats; | ||
if (node == null) { | ||
// should not happen | ||
LOG.error("Missing limit node for quota {}", limitNode); | ||
return; | ||
} | ||
synchronized (node) { | ||
limitStats = new StatsTrack(node.data); | ||
} | ||
//check the quota | ||
boolean checkCountQuota = countDiff != 0 && (limitStats.getCount() > -1 || limitStats.getCountHardLimit() > -1); | ||
boolean checkByteQuota = bytesDiff != 0 && (limitStats.getBytes() > -1 || limitStats.getByteHardLimit() > -1); | ||
|
||
if (!checkCountQuota && !checkByteQuota) { | ||
return; | ||
} | ||
|
||
//check the statPath quota | ||
String statNode = Quotas.statPath(lastPrefix); | ||
node = getZKDatabase().getNode(statNode); | ||
|
||
StatsTrack currentStats; | ||
if (node == null) { | ||
// should not happen | ||
LOG.error("Missing node for stat {}", statNode); | ||
return; | ||
} | ||
synchronized (node) { | ||
currentStats = new StatsTrack(node.data); | ||
} | ||
|
||
//check the Count Quota | ||
if (checkCountQuota) { | ||
long newCount = currentStats.getCount() + countDiff; | ||
boolean isCountHardLimit = limitStats.getCountHardLimit() > -1 ? true : false; | ||
long countLimit = isCountHardLimit ? limitStats.getCountHardLimit() : limitStats.getCount(); | ||
|
||
if (newCount > countLimit) { | ||
String msg = "Quota exceeded: " + lastPrefix + " [current count=" + newCount + ", " + (isCountHardLimit ? "hard" : "soft") + "CountLimit=" + countLimit + "]"; | ||
RATE_LOGGER.rateLimitLog(msg); | ||
if (isCountHardLimit) { | ||
throw new KeeperException.QuotaExceededException(lastPrefix); | ||
} | ||
} | ||
} | ||
|
||
//check the Byte Quota | ||
if (checkByteQuota) { | ||
long newBytes = currentStats.getBytes() + bytesDiff; | ||
boolean isByteHardLimit = limitStats.getByteHardLimit() > -1 ? true : false; | ||
long byteLimit = isByteHardLimit ? limitStats.getByteHardLimit() : limitStats.getBytes(); | ||
if (newBytes > byteLimit) { | ||
String msg = "Quota exceeded: " + lastPrefix + " [current bytes=" + newBytes + ", " + (isByteHardLimit ? "hard" : "soft") + "ByteLimit=" + byteLimit + "]"; | ||
RATE_LOGGER.rateLimitLog(msg); | ||
if (isByteHardLimit) { | ||
throw new KeeperException.QuotaExceededException(lastPrefix); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(checkQuota has 2 logical blocks with deeply nested code. Threshold is one single block per function)
@@ -46,10 +46,13 @@ | |||
import org.apache.zookeeper.KeeperException; | |||
import org.apache.zookeeper.KeeperException.Code; | |||
import org.apache.zookeeper.KeeperException.SessionExpiredException; | |||
import org.apache.zookeeper.Quotas; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkACL n-args = 6
@@ -46,10 +46,13 @@ | |||
import org.apache.zookeeper.KeeperException; | |||
import org.apache.zookeeper.KeeperException.Code; | |||
import org.apache.zookeeper.KeeperException.SessionExpiredException; | |||
import org.apache.zookeeper.Quotas; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ZooKeeperServer n-args = 8
…ibrary This is a "respin" of apache#1054, which I withdrew due to some annoying shortcomings. This changeset allows C clients to use SASL to authenticate with the ZooKeeper server. It is loosely based on patches #1 and #2 by Tom Klonikowski, at https://reviews.apache.org/r/2252/, but the result has been extensively reworked to follow the semantics of the Java client: * No SASL operations are exposed through the API; * The configuration is provided, and stored, at "handle init time"; * SASL authentication is automatically performed after each (re)connect. It introduces an optional dependency on the Cyrus SASL library, which can either be autodetected (default) or configured using the `--without-sasl`/`--with-sasl[=DIR]` flags, or -DWITH_CYRUS_SASL for CMake/Windows. `TestServerRequireClientSASLAuth.cc` has been renamed to `TestSASLAuth.cc`, and a test has been added which successfully (re)authenticates using the `DIGEST-MD5` mechanism. The code has also been used to successfully authenticate clients via `GSSAPI`/Kerberos. This commit also adds SASL support to the `cli.c` client. Co-authored-by: Tom Klonikowski <klonik_tinformatik.haw-hamburg.de> Author: Damien Diederen <[email protected]> Reviewers: Mate Szalay-Beko <[email protected]>, Norbert Kalmar <[email protected]> Closes apache#1134 from ztzg/ZOOKEEPER-1112-c-client-sasl-support-v2
…ibrary This is a "respin" of apache#1054, which I withdrew due to some annoying shortcomings. This changeset allows C clients to use SASL to authenticate with the ZooKeeper server. It is loosely based on patches #1 and #2 by Tom Klonikowski, at https://reviews.apache.org/r/2252/, but the result has been extensively reworked to follow the semantics of the Java client: * No SASL operations are exposed through the API; * The configuration is provided, and stored, at "handle init time"; * SASL authentication is automatically performed after each (re)connect. It introduces an optional dependency on the Cyrus SASL library, which can either be autodetected (default) or configured using the `--without-sasl`/`--with-sasl[=DIR]` flags, or -DWITH_CYRUS_SASL for CMake/Windows. `TestServerRequireClientSASLAuth.cc` has been renamed to `TestSASLAuth.cc`, and a test has been added which successfully (re)authenticates using the `DIGEST-MD5` mechanism. The code has also been used to successfully authenticate clients via `GSSAPI`/Kerberos. This commit also adds SASL support to the `cli.c` client. Co-authored-by: Tom Klonikowski <klonik_tinformatik.haw-hamburg.de> Author: Damien Diederen <[email protected]> Reviewers: Mate Szalay-Beko <[email protected]>, Norbert Kalmar <[email protected]> Closes apache#1134 from ztzg/ZOOKEEPER-1112-c-client-sasl-support-v2
No description provided.