rahulrane50 commented on code in PR #2219:
URL: https://github.com/apache/helix/pull/2219#discussion_r975874399
##########
helix-core/helix-core-1.0.5-SNAPSHOT.ivy:
##########
@@ -63,6 +63,7 @@ under the License.
<dependency org="org.yaml" name="snakeyaml" rev="1.30"
conf="compile->compile(default);runtime->runtime(default);default->default"/>
<dependency org="commons-logging" name="commons-logging-api" rev="1.1"
conf="compile->compile(*),master(*);runtime->runtime(*)"/>
<dependency org="io.dropwizard.metrics" name="metrics-core" rev="4.1.14"
conf="compile->compile(default);runtime->runtime(default);default->default"/>
+ <dependency org="org.xerial.snappy" name="snappy-java" rev="1.1.7"
conf="compile->compile(default);runtime->runtime(default);default->default"/>
Review Comment:
just curious, do we need this in snapshot as well? I thought just adding
this in pom.xml is good enough?
Adding it to snapshot would affect backward compatibility or not?
##########
zookeeper-api/src/test/java/org/apache/helix/zookeeper/impl/client/TestRawZkClient.java:
##########
@@ -1063,23 +1063,29 @@ public void testAsyncWriteByExpectedSession() throws
Exception {
* Tests getChildren() when there are an excessive number of children and
connection loss happens,
* the operation should terminate and exit retry loop.
*/
- @Test(timeOut = 30 * 1000L)
+ @Test
public void testGetChildrenOnLargeNumChildren() throws Exception {
final String methodName = TestHelper.getTestMethodName();
System.out.println("Start test: " + methodName);
// Create 110K children to make packet length of children exceed 4 MB
// and cause connection loss for getChildren() operation
String path = "/" + methodName;
+ int numOps = 110;
+ int numOpInOps = 1000;
+ // All the paths that are going to be created as children nodes, plus one
parent node
+ // Record paths so can be deleted at the end of the test
+ String[] nodePaths = new String[numOps * numOpInOps + 1];
+ nodePaths[numOps * numOpInOps] = path;
_zkClient.createPersistent(path);
- for (int i = 0; i < 110; i++) {
- List<Op> ops = new ArrayList<>(1000);
- for (int j = 0; j < 1000; j++) {
+ for (int i = 0; i < numOps; i++) {
+ List<Op> ops = new ArrayList<>(numOpInOps);
+ for (int j = 0; j < numOpInOps; j++) {
String childPath = path + "/" + UUID.randomUUID().toString();
- // Create ephemeral nodes so closing zkClient deletes them for cleanup
+ nodePaths[numOpInOps * i + j] = childPath;
ops.add(
- Op.create(childPath, new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE,
CreateMode.EPHEMERAL));
+ Op.create(childPath, new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE,
CreateMode.PERSISTENT));
Review Comment:
I understand for this test creating persistent node is also fine but let's
discuss if this a problem if someone is trying to create large number of
ephemeral nodes? Couple of questions :
1. Why was it working previously? Was it because jutemax buffer was 4M and
now it's 1M? because if that's the case then we should ideally keep
jutemaxbuffer for this test same as what we kept in prod and then reason out
failure/test scenario.
2. I'm still not understanding why would a large transaction log has
anything to do with data consistency. (Issue you mentioned in description). Can
you please explain?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
