You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "dcapwell (via GitHub)" <gi...@apache.org> on 2025/01/27 20:51:05 UTC
[PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
dcapwell opened a new pull request, #3835:
URL: https://github.com/apache/cassandra/pull/3835
(no comment)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1931174809
##########
test/distributed/org/apache/cassandra/fuzz/snapshots/SnapshotsTest.java:
##########
@@ -132,21 +132,21 @@ private Pair<SnapshotsHolder, Map<String, String>> generateParams()
case 1:
if (!state.truncatedSnapshots.isEmpty())
{
- keyspace = state.rs.pick(state.truncatedSnapshots.keySet()).split("\\.")[0];
+ keyspace = state.rs.pickUnorderedSet(state.truncatedSnapshots.keySet()).split("\\.")[0];
Review Comment:
in pulling all the `RandomSource` in this method got renamed to be 2 different methods
`pickUnorderedSet` and `pickOrderedSet`... unorded needs the sub type to be `Comparable` and will do a sort so the result is deterministic... `pickOrderedSet` doesn't sort as the set order should be deterministic already... this test uses `HashMap` which is non-determiistic so we need `pickUnordedSet`
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932562009
##########
test/unit/accord/utils/Gens.java:
##########
@@ -88,6 +91,63 @@ public static <T> Gen<T> oneOf(Map<Gen<T>, Integer> values)
return rs -> gen.next(rs).next(rs);
}
+ public static <T> OneOfBuilder<T> oneOf()
Review Comment:
Generally not going to make comments about unused items, given I don't know what bits will be used later in some capacity.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1931175924
##########
test/unit/accord/utils/Gens.java:
##########
@@ -336,24 +507,70 @@ public static <T> Gen<Gen<T>> mixedDistribution(T... list)
public static <T> Gen<Gen<T>> mixedDistribution(List<T> list)
{
return rs -> {
- switch (rs.nextInt(0, 2))
+ switch (rs.nextInt(0, 4))
{
case 0: // uniform
return r -> list.get(rs.nextInt(0, list.size()));
- case 1: // zipf
+ case 1: // median biased
+ int median = rs.nextInt(0, list.size());
+ return r -> list.get(r.nextBiasedInt(0, median, list.size()));
+ case 2: // zipf
List<T> array = list;
if (rs.nextBoolean())
{
array = new ArrayList<>(list);
Collections.reverse(array);
}
return pickZipf(array);
+ case 3: // random weight
+ return randomWeights(list).next(rs);
default:
throw new AssertionError();
}
};
}
+ public static <T> Gen<Gen.IntGen> mixedDistribution(int[] list)
+ {
+ return rs -> {
+ switch (rs.nextInt(0, 4))
+ {
+ case 0: // uniform
+ return r -> list[rs.nextInt(0, list.length)];
+ case 1: // median biased
+ int median = rs.nextInt(0, list.length);
+ return r -> list[r.nextBiasedInt(0, median, list.length)];
+ case 2: // zipf
+ int[] array = list;
+ if (rs.nextBoolean())
+ {
+ array = Arrays.copyOf(array, array.length);
+ reverse(array);
+ }
+ return pickZipf(array);
+ case 3: // random weight
+ return randomWeights(list).next(rs);
+ default:
+ throw new AssertionError();
+ }
+ };
+ }
+
+ /**
+ * This is a change from accord as that uses {@link accord.utils.Utils#reverse}, which doesn't exist in this forward port.
Review Comment:
this is the only change... i didn't want to bring in `Utils` and add more conflict with cep-15-accord... so in-lined... when accord merges this whole file will go away...
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932651469
##########
test/unit/accord/utils/Gens.java:
##########
@@ -88,6 +91,63 @@ public static <T> Gen<T> oneOf(Map<Gen<T>, Integer> values)
return rs -> gen.next(rs).next(rs);
}
+ public static <T> OneOfBuilder<T> oneOf()
Review Comment:
its only used in `org.apache.cassandra.tcm.log.LogStateTestBase` on cep-15-accord, but not in trunk.
I am asking Alex about this, as the usage is for TCM reconstruct of older epochs. This feature was added specifically for Accord, but we had a lot of issues with it so we migrated off it... but seems we still have the API/Tests for 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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932646548
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
+
+import java.util.Arrays;
+import java.util.function.Supplier;
+
+import accord.utils.Invariants;
+import accord.utils.RandomSource;
+
+public class Picker
+{
+ public static float[] randomWeights(RandomSource random, int length)
+ {
+ float[] weights = new float[length - 1];
+ float sum = 0;
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] = sum += random.nextFloat();
+ sum += random.nextFloat();
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] /= sum;
+ return weights;
+ }
+
+ static abstract class Weighted
+ {
+ final RandomSource random;
+ final float[] weights;
+
+ public Weighted(RandomSource random, float[] weights)
+ {
+ this.random = random;
+ this.weights = weights;
+ }
+
+
+ static float[] randomWeights(RandomSource random, float[] bias)
+ {
+ float[] weights = new float[bias.length - 1];
Review Comment:
`accord.utils.random.Picker.Weighted#pickIndex`
so, we use binary search to find what weight to select, and if the selected `float` is larger than max weight, we choose the last offset in `bias` i believe.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932657750
##########
test/unit/accord/utils/Gens.java:
##########
@@ -336,24 +507,70 @@ public static <T> Gen<Gen<T>> mixedDistribution(T... list)
public static <T> Gen<Gen<T>> mixedDistribution(List<T> list)
{
return rs -> {
- switch (rs.nextInt(0, 2))
+ switch (rs.nextInt(0, 4))
{
case 0: // uniform
return r -> list.get(rs.nextInt(0, list.size()));
- case 1: // zipf
+ case 1: // median biased
+ int median = rs.nextInt(0, list.size());
+ return r -> list.get(r.nextBiasedInt(0, median, list.size()));
+ case 2: // zipf
List<T> array = list;
if (rs.nextBoolean())
{
array = new ArrayList<>(list);
Collections.reverse(array);
}
return pickZipf(array);
+ case 3: // random weight
+ return randomWeights(list).next(rs);
default:
throw new AssertionError();
}
};
}
+ public static <T> Gen<Gen.IntGen> mixedDistribution(int[] list)
+ {
+ return rs -> {
+ switch (rs.nextInt(0, 4))
+ {
+ case 0: // uniform
+ return r -> list[rs.nextInt(0, list.length)];
+ case 1: // median biased
+ int median = rs.nextInt(0, list.length);
+ return r -> list[r.nextBiasedInt(0, median, list.length)];
+ case 2: // zipf
+ int[] array = list;
+ if (rs.nextBoolean())
+ {
+ array = Arrays.copyOf(array, array.length);
+ reverse(array);
+ }
+ return pickZipf(array);
+ case 3: // random weight
+ return randomWeights(list).next(rs);
+ default:
+ throw new AssertionError();
+ }
+ };
+ }
Review Comment:
we have a pattern of generic and primitive versions, this helps avoid extra allocations. Given that this change would force boxing I would prefer not to unify.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932651379
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
+
+import java.util.Arrays;
+import java.util.function.Supplier;
+
+import accord.utils.Invariants;
+import accord.utils.RandomSource;
+
+public class Picker
+{
+ public static float[] randomWeights(RandomSource random, int length)
+ {
+ float[] weights = new float[length - 1];
+ float sum = 0;
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] = sum += random.nextFloat();
+ sum += random.nextFloat();
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] /= sum;
+ return weights;
+ }
+
+ static abstract class Weighted
+ {
+ final RandomSource random;
+ final float[] weights;
+
+ public Weighted(RandomSource random, float[] weights)
+ {
+ this.random = random;
+ this.weights = weights;
+ }
+
+
+ static float[] randomWeights(RandomSource random, float[] bias)
+ {
+ float[] weights = new float[bias.length - 1];
Review Comment:
Yeah, like these methods don't explode, I guess, because they use the lengths of the newly created arrays in their loop conditions, but it's a little confusing that we arbitrarily memory hole the last bias in the context of this method itself.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic closed pull request #3835: CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk
URL: https://github.com/apache/cassandra/pull/3835
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932525508
##########
src/java/org/apache/cassandra/schema/ReplicationParams.java:
##########
@@ -64,6 +64,12 @@ private ReplicationParams(Class<? extends AbstractReplicationStrategy> klass, Ma
this.options = ImmutableMap.copyOf(options);
}
+ @VisibleForTesting
+ public static ReplicationParams copy(AbstractReplicationStrategy strategy)
Review Comment:
nit: What about `fromStrategy()` or `from()` something similar?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932641823
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
+
+import java.util.Arrays;
+import java.util.function.Supplier;
+
+import accord.utils.Invariants;
+import accord.utils.RandomSource;
+
+public class Picker
+{
+ public static float[] randomWeights(RandomSource random, int length)
+ {
+ float[] weights = new float[length - 1];
Review Comment:
guess so... most of the higher up APIs that end up calling this check for 0/1 and optimize around those (or block)...
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932640799
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
Review Comment:
i think there are a few things...
1) we have several testing utils that *should* have a different life cycle than accord, so likely should be moved out for better isolation
2) when cep-15-accord rebases this forward part has caused issues... the cep-15-accord history hasn't been squashed so all changes to these packages exist in cep-15-accord history, causing conflicts on rebase to trunk... to avoid this i added a commit on top of trunk last time i did the rebase to rename this package; i then removed after the rebase completed...
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932557705
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
+
+import java.util.Arrays;
+import java.util.function.Supplier;
+
+import accord.utils.Invariants;
+import accord.utils.RandomSource;
+
+public class Picker
+{
+ public static float[] randomWeights(RandomSource random, int length)
+ {
+ float[] weights = new float[length - 1];
+ float sum = 0;
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] = sum += random.nextFloat();
+ sum += random.nextFloat();
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] /= sum;
+ return weights;
+ }
+
+ static abstract class Weighted
+ {
+ final RandomSource random;
+ final float[] weights;
+
+ public Weighted(RandomSource random, float[] weights)
+ {
+ this.random = random;
+ this.weights = weights;
+ }
+
+
+ static float[] randomWeights(RandomSource random, float[] bias)
+ {
+ float[] weights = new float[bias.length - 1];
Review Comment:
Do we ignore one of the `bias` floats?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932545643
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
Review Comment:
The whole general `accord.utils` package naming is something I suppose we should address in the long term if the thing in it are really not Accord-specific. No sense in changing before `cep-15-accord` merges though, of course.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932685250
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
+
+import java.util.Arrays;
+import java.util.function.Supplier;
+
+import accord.utils.Invariants;
+import accord.utils.RandomSource;
+
+public class Picker
+{
+ public static float[] randomWeights(RandomSource random, int length)
+ {
+ float[] weights = new float[length - 1];
+ float sum = 0;
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] = sum += random.nextFloat();
+ sum += random.nextFloat();
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] /= sum;
+ return weights;
+ }
+
+ static abstract class Weighted
+ {
+ final RandomSource random;
+ final float[] weights;
+
+ public Weighted(RandomSource random, float[] weights)
+ {
+ this.random = random;
+ this.weights = weights;
+ }
+
+
+ static float[] randomWeights(RandomSource random, float[] bias)
+ {
+ float[] weights = new float[bias.length - 1];
Review Comment:
its very leaky... you must use it "correctly" and only once place in the code knows what that means...
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1931173328
##########
src/java/org/apache/cassandra/config/CassandraRelevantProperties.java:
##########
@@ -554,7 +554,7 @@ public enum CassandraRelevantProperties
TCM_USE_NO_OP_REPLICATOR("cassandra.test.use_no_op_replicator", "false"),
TEST_BBFAILHELPER_ENABLED("test.bbfailhelper.enabled"),
- TEST_BLOB_SHARED_SEED("cassandra.test.blob.shared.seed"),
+ TEST_BLOB_SHARED_SEED("cassandra.test.blob.shared.seed", "42"),
Review Comment:
current logic used current time, which meant that tests were non-deterministic.... the seed *must* be set in order to be able to reproduce issues
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932634898
##########
src/java/org/apache/cassandra/config/CassandraRelevantProperties.java:
##########
@@ -554,7 +554,7 @@ public enum CassandraRelevantProperties
TCM_USE_NO_OP_REPLICATOR("cassandra.test.use_no_op_replicator", "false"),
TEST_BBFAILHELPER_ENABLED("test.bbfailhelper.enabled"),
- TEST_BLOB_SHARED_SEED("cassandra.test.blob.shared.seed"),
+ TEST_BLOB_SHARED_SEED("cassandra.test.blob.shared.seed", "42"),
Review Comment:
so, the tests use `qt()` and controls the seed for the test. The blob logic has a giant blob that is shared by all tests, and uses a seed to populate the bytes... this seed is decoupled from the test seed, and would use `currentTimeMillis`; this caused the tests to be non-deterministic and not repeatable when errors were found.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932647230
##########
test/unit/accord/utils/Property.java:
##########
@@ -555,7 +555,7 @@ default void checkPostconditions(State state, Result expected,
default void process(State state, SystemUnderTest sut) throws Throwable
{
checkPostconditions(state, apply(state),
- sut, run(sut));
+ sut, run(sut));
Review Comment:
nit: Might as well one-liner this one if we're going to reformat
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932552089
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
+
+import java.util.Arrays;
+import java.util.function.Supplier;
+
+import accord.utils.Invariants;
+import accord.utils.RandomSource;
+
+public class Picker
+{
+ public static float[] randomWeights(RandomSource random, int length)
+ {
+ float[] weights = new float[length - 1];
Review Comment:
If we have a `length` of 1, is the method supposed to just return an empty array?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932652250
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
+
+import java.util.Arrays;
+import java.util.function.Supplier;
+
+import accord.utils.Invariants;
+import accord.utils.RandomSource;
+
+public class Picker
+{
+ public static float[] randomWeights(RandomSource random, int length)
+ {
+ float[] weights = new float[length - 1];
+ float sum = 0;
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] = sum += random.nextFloat();
+ sum += random.nextFloat();
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] /= sum;
+ return weights;
+ }
+
+ static abstract class Weighted
+ {
+ final RandomSource random;
+ final float[] weights;
+
+ public Weighted(RandomSource random, float[] weights)
+ {
+ this.random = random;
+ this.weights = weights;
+ }
+
+
+ static float[] randomWeights(RandomSource random, float[] bias)
+ {
+ float[] weights = new float[bias.length - 1];
+ float sum = 0;
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] = sum += random.nextFloat() * bias[i];
+ sum += random.nextFloat() * bias[weights.length];
+ for (int i = 0 ; i < weights.length ; ++i)
+ weights[i] /= sum;
Review Comment:
nit: Also, this method seems to normalize the random weights, but that's not reflected in the name.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932659411
##########
test/unit/accord/utils/Property.java:
##########
@@ -555,7 +555,7 @@ default void checkPostconditions(State state, Result expected,
default void process(State state, SystemUnderTest sut) throws Throwable
{
checkPostconditions(state, apply(state),
- sut, run(sut));
+ sut, run(sut));
Review Comment:
this is a personal style choice. `state, apply(state)` is everything going on for the `state`, and `sut, run(sut)` is the same for `sut`... i felt this was easier to read as 2 lines rather than 1.
If you do not agree, i can normalize by making one line; just giving my argument
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932545643
##########
test/unit/accord/utils/random/Picker.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 accord.utils.random;
Review Comment:
The whole general `accord.utils` package naming is something I suppose we should address in the long term if the things in it are really not Accord-specific. No sense in changing before `cep-15-accord` merges though, of course.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1931178735
##########
test/unit/org/apache/cassandra/utils/CassandraGenerators.java:
##########
@@ -183,29 +214,304 @@ public static Gen<String> sstableFormatNames()
return SourceDSL.arbitrary().pick("big", "bti");
}
+ public static Gen<SSTableFormat<?, ?>> sstableFormat()
+ {
+ // make sure ordering is determanstic, else repeatability breaks
+ NavigableMap<String, SSTableFormat<?, ?>> formats = new TreeMap<>(DatabaseDescriptor.getSSTableFormats());
+ return SourceDSL.arbitrary().pick(new ArrayList<>(formats.values()));
+ }
+
+ public static class AbstractReplicationStrategyBuilder
+ {
+ public enum Strategy
+ {
+ Simple(true),
+ NetworkTopology(true),
+ Local(false),
+ Meta(false);
+
+ public final boolean userAllowed;
+
+ Strategy(boolean userAllowed)
+ {
+ this.userAllowed = userAllowed;
+ }
+ }
+
+ private Gen<Strategy> strategyGen = SourceDSL.arbitrary().enumValues(Strategy.class);
+ private Gen<String> keyspaceNameGen = KEYSPACE_NAME_GEN;
+ private Gen<Integer> rfGen = SourceDSL.integers().between(1, 3);
+ private Gen<List<String>> networkTopologyDCGen = rs -> {
+ Gen<Integer> numDcsGen = SourceDSL.integers().between(1, 3);
+ Gen<String> nameGen = IDENTIFIER_GEN;
+ Set<String> dcs = new HashSet<>();
+ int targetSize = numDcsGen.generate(rs);
+ while (dcs.size() != targetSize)
+ dcs.add(nameGen.generate(rs));
+ List<String> ordered = new ArrayList<>(dcs);
+ ordered.sort(Comparator.naturalOrder());
+ return ordered;
+ };
+
+ public AbstractReplicationStrategyBuilder withKeyspace(Gen<String> keyspaceNameGen)
+ {
+ this.keyspaceNameGen = keyspaceNameGen;
+ return this;
+ }
+
+ public AbstractReplicationStrategyBuilder withKeyspace(String keyspace)
+ {
+ this.keyspaceNameGen = i -> keyspace;
+ return this;
+ }
+
+ public AbstractReplicationStrategyBuilder withUserAllowed()
+ {
+ List<Strategy> allowed = Stream.of(Strategy.values()).filter(s -> s.userAllowed).collect(Collectors.toList());
+ strategyGen = SourceDSL.arbitrary().pick(allowed);
+ return this;
+ }
+
+ public AbstractReplicationStrategyBuilder withRf(Gen<Integer> rfGen)
+ {
+ this.rfGen = rfGen;
+ return this;
+ }
+
+ public AbstractReplicationStrategyBuilder withRf(int rf)
+ {
+ this.rfGen = i -> rf;
+ return this;
+ }
+
+ public AbstractReplicationStrategyBuilder withDatacenters(Gen<List<String>> networkTopologyDCGen)
+ {
+ this.networkTopologyDCGen = networkTopologyDCGen;
+ return this;
+ }
+
+ public AbstractReplicationStrategyBuilder withDatacenters(String first, String... rest)
+ {
+ if (rest.length == 0)
+ {
+ this.networkTopologyDCGen = i -> Collections.singletonList(first);
+ }
+ else
+ {
+ List<String> all = new ArrayList<>(rest.length + 1);
+ all.add(first);
+ all.addAll(Arrays.asList(rest));
+ this.networkTopologyDCGen = i -> all;
+ }
+ return this;
+ }
+
+ public Gen<AbstractReplicationStrategy> build()
+ {
+ return rs -> {
+ Strategy strategy = strategyGen.generate(rs);
+ switch (strategy)
+ {
+ case Simple:
+ return new SimpleStrategy(keyspaceNameGen.generate(rs),
+ ImmutableMap.of(SimpleStrategy.REPLICATION_FACTOR, rfGen.generate(rs).toString()));
+ case NetworkTopology:
+ ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
+ List<String> names = networkTopologyDCGen.generate(rs);
+ for (String name : names)
+ builder.put(name, rfGen.generate(rs).toString());
+ ImmutableMap<String, String> map = builder.build();
+ return new TestableNetworkTopologyStrategy(keyspaceNameGen.generate(rs), map);
+ case Meta:
+ return new MetaStrategy(keyspaceNameGen.generate(rs), ImmutableMap.of());
+ case Local:
+ return new LocalStrategy(keyspaceNameGen.generate(rs), ImmutableMap.of());
+ default:
+ throw new UnsupportedOperationException(strategy.name());
+ }
+ };
+ }
+ }
+
+ public static class TestableNetworkTopologyStrategy extends NetworkTopologyStrategy
Review Comment:
`NetworkTopologyStrategy` validates the datacenters match a config, but that makes it a bit harder to use in some contexts... so this one avoids validating datacenters!
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932632830
##########
test/unit/accord/utils/Gens.java:
##########
@@ -336,24 +507,70 @@ public static <T> Gen<Gen<T>> mixedDistribution(T... list)
public static <T> Gen<Gen<T>> mixedDistribution(List<T> list)
{
return rs -> {
- switch (rs.nextInt(0, 2))
+ switch (rs.nextInt(0, 4))
{
case 0: // uniform
return r -> list.get(rs.nextInt(0, list.size()));
- case 1: // zipf
+ case 1: // median biased
+ int median = rs.nextInt(0, list.size());
+ return r -> list.get(r.nextBiasedInt(0, median, list.size()));
+ case 2: // zipf
List<T> array = list;
if (rs.nextBoolean())
{
array = new ArrayList<>(list);
Collections.reverse(array);
}
return pickZipf(array);
+ case 3: // random weight
+ return randomWeights(list).next(rs);
default:
throw new AssertionError();
}
};
}
+ public static <T> Gen<Gen.IntGen> mixedDistribution(int[] list)
+ {
+ return rs -> {
+ switch (rs.nextInt(0, 4))
+ {
+ case 0: // uniform
+ return r -> list[rs.nextInt(0, list.length)];
+ case 1: // median biased
+ int median = rs.nextInt(0, list.length);
+ return r -> list[r.nextBiasedInt(0, median, list.length)];
+ case 2: // zipf
+ int[] array = list;
+ if (rs.nextBoolean())
+ {
+ array = Arrays.copyOf(array, array.length);
+ reverse(array);
+ }
+ return pickZipf(array);
+ case 3: // random weight
+ return randomWeights(list).next(rs);
+ default:
+ throw new AssertionError();
+ }
+ };
+ }
Review Comment:
There are some things in this class that we might able to slim down with some tweaking of types. This method might be able to go away with this patch, for instance:
```
Subject: [PATCH] DRY up mixedDistribution()
---
Index: test/unit/accord/utils/Gens.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/unit/accord/utils/Gens.java b/test/unit/accord/utils/Gens.java
--- a/test/unit/accord/utils/Gens.java (revision 7bc06a322c211f52fbb6df60f2058a32cd57dafc)
+++ b/test/unit/accord/utils/Gens.java (date 1738086893079)
@@ -331,15 +331,16 @@
throw new IllegalArgumentException("Range is too large; min=" + minInclusive + ", max=" + maxExclusive);
if (numBuckets <= 0 || numBuckets > domainSize)
throw new IllegalArgumentException("Num buckets must be between 1 and " + domainSize + "; given " + numBuckets);
- int[] bucket, indexes;
- bucket = new int[numBuckets];
+ Integer[] bucket, indexes;
+ bucket = new Integer[numBuckets];
int delta = domainSize / numBuckets;
for (int i = 0; i < numBuckets; i++)
bucket[i] = minInclusive + i * delta;
- indexes = IntStream.range(0, bucket.length).toArray();
- Gen<Gen.IntGen> indexDistro = mixedDistribution(indexes);
+
+ indexes = (Integer[]) IntStream.range(0, bucket.length).boxed().toArray();
+ Gen<Gen<Integer>> indexDistro = mixedDistribution(indexes);
return rs -> {
- Gen.IntGen indexGen = indexDistro.next(rs);
+ Gen<Integer> indexGen = indexDistro.next(rs);
switch (rs.nextInt(0, 2))
{
case 0: // uniform
@@ -530,30 +531,9 @@
};
}
- public static <T> Gen<Gen.IntGen> mixedDistribution(int[] list)
+ public static Gen<Gen<Integer>> mixedDistribution(Integer[] list)
{
- return rs -> {
- switch (rs.nextInt(0, 4))
- {
- case 0: // uniform
- return r -> list[rs.nextInt(0, list.length)];
- case 1: // median biased
- int median = rs.nextInt(0, list.length);
- return r -> list[r.nextBiasedInt(0, median, list.length)];
- case 2: // zipf
- int[] array = list;
- if (rs.nextBoolean())
- {
- array = Arrays.copyOf(array, array.length);
- reverse(array);
- }
- return pickZipf(array);
- case 3: // random weight
- return randomWeights(list).next(rs);
- default:
- throw new AssertionError();
- }
- };
+ return mixedDistribution(Arrays.asList(list));
}
/**
```
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1931180773
##########
test/unit/org/apache/cassandra/utils/CassandraGenerators.java:
##########
@@ -650,13 +1024,32 @@ public static Gen<IPartitioner> partitioners()
.flatMap(SupportedPartitioners::partitioner);
}
+
public static Gen<IPartitioner> nonLocalPartitioners()
{
return SourceDSL.arbitrary().enumValues(SupportedPartitioners.class)
.assuming(p -> p != SupportedPartitioners.Local)
.flatMap(SupportedPartitioners::partitioner);
}
+ /**
+ * For {@link LocalPartitioner} it can have a very complex type which can lead to generating data larger than
+ * allowed in a primary key. If a test needs to filter out those cases, can just
+ * {@code .map(CassandraGenerators::simplify)} to resolve.
+ */
+ public static IPartitioner simplify(IPartitioner partitioner)
Review Comment:
in a follow up patch ill prob remove this... `LocalPartitioner` has caused flakyness for several tests in cep-15-accord so plan to refactor that logic to be less problematic, which lets us drop this function... keeping for now just to be in-sync
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1931182077
##########
test/unit/org/apache/cassandra/utils/Generators.java:
##########
@@ -466,7 +535,7 @@ private static final class LazySharedBlob
static
{
- long blobSeed = TEST_BLOB_SHARED_SEED.getLong(System.currentTimeMillis());
+ long blobSeed = TEST_BLOB_SHARED_SEED.getLong();
Review Comment:
this default was never correct, as it made tests non-reproducable
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932513235
##########
src/java/org/apache/cassandra/config/CassandraRelevantProperties.java:
##########
@@ -554,7 +554,7 @@ public enum CassandraRelevantProperties
TCM_USE_NO_OP_REPLICATOR("cassandra.test.use_no_op_replicator", "false"),
TEST_BBFAILHELPER_ENABLED("test.bbfailhelper.enabled"),
- TEST_BLOB_SHARED_SEED("cassandra.test.blob.shared.seed"),
+ TEST_BLOB_SHARED_SEED("cassandra.test.blob.shared.seed", "42"),
Review Comment:
Not sure I follow. Is the problem that we just didn't output the seed in whatever failure message was produced?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932644678
##########
src/java/org/apache/cassandra/config/CassandraRelevantProperties.java:
##########
@@ -554,7 +554,7 @@ public enum CassandraRelevantProperties
TCM_USE_NO_OP_REPLICATOR("cassandra.test.use_no_op_replicator", "false"),
TEST_BBFAILHELPER_ENABLED("test.bbfailhelper.enabled"),
- TEST_BLOB_SHARED_SEED("cassandra.test.blob.shared.seed"),
+ TEST_BLOB_SHARED_SEED("cassandra.test.blob.shared.seed", "42"),
Review Comment:
Yeah, not something I'd expect to unify in this patch.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20253: Forward port randomized testing improvements from cep-15-accord to trunk [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3835:
URL: https://github.com/apache/cassandra/pull/3835#discussion_r1932635802
##########
src/java/org/apache/cassandra/schema/ReplicationParams.java:
##########
@@ -64,6 +64,12 @@ private ReplicationParams(Class<? extends AbstractReplicationStrategy> klass, Ma
this.options = ImmutableMap.copyOf(options);
}
+ @VisibleForTesting
+ public static ReplicationParams copy(AbstractReplicationStrategy strategy)
Review Comment:
`fromStrategy` works for me
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org