You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/07/08 08:42:32 UTC
[GitHub] [pulsar] mattisonchao opened a new pull request, #16473: [Don't Merge] Test master branch test.
mattisonchao opened a new pull request, #16473:
URL: https://github.com/apache/pulsar/pull/16473
### Motivation
Test master branch test.
### Modifications
- Add catch for MockZookeeper.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
This change is a trivial rework / code cleanup without any test coverage.
*(or)*
This change is already covered by existing tests, such as *(please describe tests)*.
*(or)*
This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end deployment with large payloads (10MB)*
- *Extended integration test for recovery after broker failure*
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (yes / no)
- The public API: (yes / no)
- The schema: (yes / no / don't know)
- The default values of configurations: (yes / no)
- The wire protocol: (yes / no)
- The rest endpoints: (yes / no)
- The admin cli options: (yes / no)
- Anything that affects deployment: (yes / no / don't know)
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [x] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #16473:
URL: https://github.com/apache/pulsar/pull/16473#issuecomment-1179037340
The `mockZookeeper` lock may have some problems, we still need to follow up on it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao closed pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
mattisonchao closed pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
URL: https://github.com/apache/pulsar/pull/16473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #16473:
URL: https://github.com/apache/pulsar/pull/16473#discussion_r917196729
##########
testmocks/src/main/java/org/apache/zookeeper/MockZooKeeper.java:
##########
@@ -532,6 +540,10 @@ public void getChildren(final String path, final Watcher watcher, final Children
if (watcher != null) {
watchers.put(path, watcher);
}
+ } catch (Throwable ex) {
+ log.error("get children : {} error", path, ex);
+ cb.processResult(KeeperException.Code.SYSTEMERROR.intValue(), path, ctx, null);
+ return;
Review Comment:
Better to move line 551 to `try` block and delete `return 546`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #16473:
URL: https://github.com/apache/pulsar/pull/16473#discussion_r917964724
##########
pom.xml:
##########
@@ -100,7 +100,7 @@ flexible messaging model and an intuitive client API.</description>
--add-opens java.management/sun.management=ALL-UNNAMED <!--JvmDefaultGCMetricsLogger-->
</test.additional.args>
<testReuseFork>true</testReuseFork>
- <testForkCount>4</testForkCount>
+ <testForkCount>0</testForkCount>
Review Comment:
this is not related to the pull. please revert
##########
build/run_unit_group.sh:
##########
@@ -64,7 +64,7 @@ alias echo='{ [[ $- =~ .*x.* ]] && trace_enabled=1 || trace_enabled=0; set +x; }
# Test Groups -- start --
function test_group_broker_group_1() {
- mvn_test -pl pulsar-broker -Dgroups='broker' -DtestReuseFork=true -DskipAfterFailureCount=1
+ mvn_test -pl pulsar-broker -Dgroups='broker' -DtestReuseFork=true -DskipAfterFailureCount=1 -DforkCount=0
Review Comment:
this is not related to the pull. please revert
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #16473:
URL: https://github.com/apache/pulsar/pull/16473#discussion_r917196775
##########
testmocks/src/main/java/org/apache/zookeeper/MockZooKeeper.java:
##########
@@ -633,6 +645,10 @@ public void getChildren(final String path, boolean watcher, final Children2Callb
String child = relativePath.split("/", 2)[0];
children.add(child);
});
+ } catch (Throwable ex) {
+ log.error("get children : {} error", path, ex);
+ cb.processResult(KeeperException.Code.SYSTEMERROR.intValue(), path, ctx, null, null);
+ return;
Review Comment:
The same with above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- closed pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
Technoboy- closed pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
URL: https://github.com/apache/pulsar/pull/16473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #16473:
URL: https://github.com/apache/pulsar/pull/16473#discussion_r917200914
##########
testmocks/src/main/java/org/apache/zookeeper/MockZooKeeper.java:
##########
@@ -633,6 +645,10 @@ public void getChildren(final String path, boolean watcher, final Children2Callb
String child = relativePath.split("/", 2)[0];
children.add(child);
});
+ } catch (Throwable ex) {
+ log.error("get children : {} error", path, ex);
+ cb.processResult(KeeperException.Code.SYSTEMERROR.intValue(), path, ctx, null, null);
+ return;
Review Comment:
Fixed.
##########
testmocks/src/main/java/org/apache/zookeeper/MockZooKeeper.java:
##########
@@ -532,6 +540,10 @@ public void getChildren(final String path, final Watcher watcher, final Children
if (watcher != null) {
watchers.put(path, watcher);
}
+ } catch (Throwable ex) {
+ log.error("get children : {} error", path, ex);
+ cb.processResult(KeeperException.Code.SYSTEMERROR.intValue(), path, ctx, null);
+ return;
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- closed pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
Technoboy- closed pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
URL: https://github.com/apache/pulsar/pull/16473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- closed pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
Technoboy- closed pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
URL: https://github.com/apache/pulsar/pull/16473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #16473:
URL: https://github.com/apache/pulsar/pull/16473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #16473:
URL: https://github.com/apache/pulsar/pull/16473#discussion_r917989685
##########
build/run_unit_group.sh:
##########
@@ -64,7 +64,7 @@ alias echo='{ [[ $- =~ .*x.* ]] && trace_enabled=1 || trace_enabled=0; set +x; }
# Test Groups -- start --
function test_group_broker_group_1() {
- mvn_test -pl pulsar-broker -Dgroups='broker' -DtestReuseFork=true -DskipAfterFailureCount=1
+ mvn_test -pl pulsar-broker -Dgroups='broker' -DtestReuseFork=true -DskipAfterFailureCount=1 -DforkCount=0
Review Comment:
yes, right. We find the test failed not related to zk(may be fixed by this patch), but not reproducible locally. So we try to modify some parameters that may affect them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #16473: [fix][test] Catch exception when update data in mockZookeeper
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #16473:
URL: https://github.com/apache/pulsar/pull/16473#discussion_r918449331
##########
pom.xml:
##########
@@ -100,7 +100,7 @@ flexible messaging model and an intuitive client API.</description>
--add-opens java.management/sun.management=ALL-UNNAMED <!--JvmDefaultGCMetricsLogger-->
</test.additional.args>
<testReuseFork>true</testReuseFork>
- <testForkCount>4</testForkCount>
+ <testForkCount>0</testForkCount>
Review Comment:
Fixed
##########
build/run_unit_group.sh:
##########
@@ -64,7 +64,7 @@ alias echo='{ [[ $- =~ .*x.* ]] && trace_enabled=1 || trace_enabled=0; set +x; }
# Test Groups -- start --
function test_group_broker_group_1() {
- mvn_test -pl pulsar-broker -Dgroups='broker' -DtestReuseFork=true -DskipAfterFailureCount=1
+ mvn_test -pl pulsar-broker -Dgroups='broker' -DtestReuseFork=true -DskipAfterFailureCount=1 -DforkCount=0
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org