You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "ifesdjeen (via GitHub)" <gi...@apache.org> on 2023/11/29 16:56:05 UTC
[PR] Remove notion of Modification [cassandra-harry]
ifesdjeen opened a new pull request, #23:
URL: https://github.com/apache/cassandra-harry/pull/23
Patch for CASSANDRA-19117 by Alex Petrov; reviewed by Abe Ratnofsky
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414610914
##########
harry-core/src/harry/model/DescriptorSelectorBuilder.java:
##########
@@ -33,7 +33,7 @@ public class DescriptorSelectorBuilder implements Configuration.CDSelectorConfig
{
private Function<SchemaSpec, OpSelectors.ColumnSelector> columnSelectorFactory;
private OpSelectors.OperationSelector operationSelector = DEFAULT_OP_SELECTOR;
- private Distribution numberOfRowsDistribution = new Distribution.ScaledDistribution(2, 30);
+ private Distribution modificationsPerLts = new Distribution.ScaledDistribution(2, 30);
private Distribution numberOfModificationsDistribution = new Distribution.ScaledDistribution(1, 3);
private int maxPartitionSize = Integer.MAX_VALUE;
private Function<SchemaSpec, int[]> fractionsSupplier = null;
Review Comment:
The initial idea was to use HierarchicalDescriptorSelector: sort of to make "predictable" partitions, where parts of CK would repeat.
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412435081
##########
harry-core/src/harry/model/DescriptorSelectorBuilder.java:
##########
@@ -72,9 +72,9 @@ public DescriptorSelectorBuilder setOperationSelector(OpSelectors.OperationSelec
* In a given modification, we are only able to visit as many rows as there are rows in the partition, so
* we'll always be limited by this.
**/
- public DescriptorSelectorBuilder setRowsPerModificationDistribution(Distribution numberOfRowsDistribution)
+ public DescriptorSelectorBuilder setModificationsPerLts(Distribution numberOfRowsDistribution)
Review Comment:
```suggestion
public DescriptorSelectorBuilder setModificationsPerLts(Distribution this.modificationsPerLts)
```
?
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414611597
##########
harry-core/test/harry/model/OpSelectorsTest.java:
##########
@@ -73,19 +72,16 @@ public void testRowDataDescriptorSupplier()
for (int lts = 0; lts < RUNS; lts++)
{
long pd = pdSupplier.pd(lts);
- for (int m = 0; m < descriptorSelector.numberOfModifications(lts); m++)
+
+ int opsPerMod = descriptorSelector.operationsPerLts(lts);
Review Comment:
Great point!
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412436415
##########
harry-core/src/harry/model/DescriptorSelectorBuilder.java:
##########
@@ -33,7 +33,7 @@ public class DescriptorSelectorBuilder implements Configuration.CDSelectorConfig
{
private Function<SchemaSpec, OpSelectors.ColumnSelector> columnSelectorFactory;
private OpSelectors.OperationSelector operationSelector = DEFAULT_OP_SELECTOR;
- private Distribution numberOfRowsDistribution = new Distribution.ScaledDistribution(2, 30);
+ private Distribution modificationsPerLts = new Distribution.ScaledDistribution(2, 30);
private Distribution numberOfModificationsDistribution = new Distribution.ScaledDistribution(1, 3);
private int maxPartitionSize = Integer.MAX_VALUE;
private Function<SchemaSpec, int[]> fractionsSupplier = null;
Review Comment:
nit: What does `fractionsSupplier` do?
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412521262
##########
harry-core/test/harry/operations/RelationTest.java:
##########
@@ -183,12 +183,12 @@ public BitSet columnMask(long pd, long lts, long opId, OpSelectors.OperationKind
throw new RuntimeException("not implemented");
}
- public long rowId(long pd, long lts, long cd)
+ public long opId(long pd, long lts, long cd)
{
return 0;
}
- public long modificationId(long pd, long cd, long lts, long vd, int col)
+ public long operationId(long pd, long cd, long lts, long vd, int col)
Review Comment:
unused now?
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412438172
##########
harry-core/src/harry/model/DescriptorSelectorBuilder.java:
##########
@@ -72,9 +72,9 @@ public DescriptorSelectorBuilder setOperationSelector(OpSelectors.OperationSelec
* In a given modification, we are only able to visit as many rows as there are rows in the partition, so
* we'll always be limited by this.
**/
- public DescriptorSelectorBuilder setRowsPerModificationDistribution(Distribution numberOfRowsDistribution)
+ public DescriptorSelectorBuilder setModificationsPerLts(Distribution numberOfRowsDistribution)
Review Comment:
I guess we've named it `operationsPerLtsDistribution` elsewhere, so I guess that's fine too if we want to be explicit.
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414603226
##########
harry-core/src/harry/model/sut/injvm/InJVMTokenAwareVisitExecutor.java:
##########
@@ -72,44 +78,43 @@ public InJVMTokenAwareVisitExecutor(Run run,
}
@Override
- protected void executeAsyncWithRetries(long lts, long pd, CompletableFuture<Object[][]> future, CompiledStatement statement)
- {
- executeAsyncWithRetries(lts, pd, future, statement, 0);
- }
-
- private void executeAsyncWithRetries(long lts, long pd, CompletableFuture<Object[][]> future, CompiledStatement statement, int retries)
+ protected Object[][] executeWithRetries(long lts, long pd, CompiledStatement statement)
{
if (sut.isShutdown())
throw new IllegalStateException("System under test is shut down");
- if (retries > this.MAX_RETRIES)
- throw new IllegalStateException(String.format("Can not execute statement %s after %d retries", statement, retries));
+ int retries = 0;
- Object[] pk = schema.inflatePartitionKey(pd);
+ Object[] pk = schema.inflatePartitionKey(pd);
List<TokenPlacementModel.Node> replicas = getRing().replicasFor(TokenUtil.token(ByteUtils.compose(ByteUtils.objectsToBytes(pk))));
-
- TokenPlacementModel.Node replica = replicas.get((int) (lts % replicas.size()));
- if (cl == SystemUnderTest.ConsistencyLevel.NODE_LOCAL)
- {
- future.complete(executeNodeLocal(statement.cql(), replica, statement.bindings()));
- }
- else
+ while (retries++ < MAX_RETRIES)
{
- CompletableFuture.supplyAsync(() -> sut.cluster
- .stream()
- .filter((n) -> n.config().broadcastAddress().toString().contains(replica.id))
- .findFirst()
- .get()
- .coordinator()
- .execute(statement.cql(), InJvmSut.toApiCl(cl), statement.bindings()), executor)
- .whenComplete((res, t) ->
- {
- if (t != null)
- executor.schedule(() -> executeAsyncWithRetries(lts, pd, future, statement, retries + 1), 1, TimeUnit.SECONDS);
- else
- future.complete(res);
- });
+ try
+ {
+ TokenPlacementModel.Node replica = replicas.get((int) (lts % replicas.size()));
+ if (cl == SystemUnderTest.ConsistencyLevel.NODE_LOCAL)
+ {
+ return executeNodeLocal(statement.cql(), replica, statement.bindings());
+ }
+ else
+ {
+ return sut.cluster
+ .stream()
+ .filter((n) -> n.config().broadcastAddress().toString().contains(replica.id))
+ .findFirst()
+ .get()
+ .coordinator()
+ .execute(statement.cql(), InJvmSut.toApiCl(cl), statement.bindings());
+ }
+ }
+ catch (Throwable t)
+ {
+ int delaySecs = 1;
+ logger.error(String.format("Caught message while trying to execute: %s. Scheduled to retry in %s seconds", statement, delaySecs), t);
+ Uninterruptibles.sleepUninterruptibly(delaySecs, TimeUnit.SECONDS);
+ }
}
+ throw new IllegalStateException(String.format("Can not execute statement %s after %d retries", statement, retries));
Review Comment:
Let's say this was the simplest way to know that a query _has_ in fact finished. This is a barebone model, and probably should be improved upon. But at least it gives you some ground to stand upon.
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414612629
##########
harry-core/test/harry/operations/RelationTest.java:
##########
@@ -183,12 +183,12 @@ public BitSet columnMask(long pd, long lts, long opId, OpSelectors.OperationKind
throw new RuntimeException("not implemented");
}
- public long rowId(long pd, long lts, long cd)
+ public long opId(long pd, long lts, long cd)
{
return 0;
}
- public long modificationId(long pd, long cd, long lts, long vd, int col)
+ public long operationId(long pd, long cd, long lts, long vd, int col)
Review Comment:
Good point! Removed this and some more.
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412429423
##########
harry-core/src/harry/model/sut/injvm/InJVMTokenAwareVisitExecutor.java:
##########
@@ -72,44 +78,43 @@ public InJVMTokenAwareVisitExecutor(Run run,
}
@Override
- protected void executeAsyncWithRetries(long lts, long pd, CompletableFuture<Object[][]> future, CompiledStatement statement)
- {
- executeAsyncWithRetries(lts, pd, future, statement, 0);
- }
-
- private void executeAsyncWithRetries(long lts, long pd, CompletableFuture<Object[][]> future, CompiledStatement statement, int retries)
+ protected Object[][] executeWithRetries(long lts, long pd, CompiledStatement statement)
{
if (sut.isShutdown())
throw new IllegalStateException("System under test is shut down");
- if (retries > this.MAX_RETRIES)
- throw new IllegalStateException(String.format("Can not execute statement %s after %d retries", statement, retries));
+ int retries = 0;
- Object[] pk = schema.inflatePartitionKey(pd);
+ Object[] pk = schema.inflatePartitionKey(pd);
List<TokenPlacementModel.Node> replicas = getRing().replicasFor(TokenUtil.token(ByteUtils.compose(ByteUtils.objectsToBytes(pk))));
-
- TokenPlacementModel.Node replica = replicas.get((int) (lts % replicas.size()));
- if (cl == SystemUnderTest.ConsistencyLevel.NODE_LOCAL)
- {
- future.complete(executeNodeLocal(statement.cql(), replica, statement.bindings()));
- }
- else
+ while (retries++ < MAX_RETRIES)
{
- CompletableFuture.supplyAsync(() -> sut.cluster
- .stream()
- .filter((n) -> n.config().broadcastAddress().toString().contains(replica.id))
- .findFirst()
- .get()
- .coordinator()
- .execute(statement.cql(), InJvmSut.toApiCl(cl), statement.bindings()), executor)
- .whenComplete((res, t) ->
- {
- if (t != null)
- executor.schedule(() -> executeAsyncWithRetries(lts, pd, future, statement, retries + 1), 1, TimeUnit.SECONDS);
- else
- future.complete(res);
- });
+ try
+ {
+ TokenPlacementModel.Node replica = replicas.get((int) (lts % replicas.size()));
+ if (cl == SystemUnderTest.ConsistencyLevel.NODE_LOCAL)
+ {
+ return executeNodeLocal(statement.cql(), replica, statement.bindings());
+ }
+ else
+ {
+ return sut.cluster
+ .stream()
+ .filter((n) -> n.config().broadcastAddress().toString().contains(replica.id))
+ .findFirst()
+ .get()
+ .coordinator()
+ .execute(statement.cql(), InJvmSut.toApiCl(cl), statement.bindings());
+ }
+ }
+ catch (Throwable t)
+ {
+ int delaySecs = 1;
+ logger.error(String.format("Caught message while trying to execute: %s. Scheduled to retry in %s seconds", statement, delaySecs), t);
+ Uninterruptibles.sleepUninterruptibly(delaySecs, TimeUnit.SECONDS);
+ }
}
+ throw new IllegalStateException(String.format("Can not execute statement %s after %d retries", statement, retries));
Review Comment:
So tl;dr we made this synchronous and added some logging for visibility? I'll figure out why I suppose as I look at its callers...
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414602598
##########
harry-core/src/harry/dsl/HistoryBuilder.java:
##########
@@ -361,32 +350,22 @@ public HistoryBuilder finish()
void addSteps(List<Step> steps)
{
- List<ReplayingVisitor.Batch> batches = new ArrayList<>();
- Counter m = new Counter();
+ List<ReplayingVisitor.Operation> operations = new ArrayList<>();
Counter opId = new Counter();
for (Step step : steps)
{
- batches.add(step.toBatch(pd, lts, m.get(), opId::getAndIncrement));
Review Comment:
yes! Is a small simplificaiton, but makes things much simpler when working on history builder, less indirection.
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen merged PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414608492
##########
harry-core/src/harry/model/DescriptorSelectorBuilder.java:
##########
@@ -72,9 +72,9 @@ public DescriptorSelectorBuilder setOperationSelector(OpSelectors.OperationSelec
* In a given modification, we are only able to visit as many rows as there are rows in the partition, so
* we'll always be limited by this.
**/
- public DescriptorSelectorBuilder setRowsPerModificationDistribution(Distribution numberOfRowsDistribution)
+ public DescriptorSelectorBuilder setModificationsPerLts(Distribution numberOfRowsDistribution)
Review Comment:
Renamed to `setOperationsPerLts`
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414602106
##########
harry-core/src/harry/core/Configuration.java:
##########
@@ -885,20 +885,20 @@ public DefaultCDSelectorConfiguration build()
public static class DefaultCDSelectorConfiguration implements CDSelectorConfiguration
{
public final DistributionConfig modifications_per_lts;
- public final DistributionConfig rows_per_modification;
+ public final DistributionConfig operations_per_lts;
public final int max_partition_size;
public final Map<OpSelectors.OperationKind, Integer> operation_kind_weights;
public final Map<OpSelectors.OperationKind, long[]> column_mask_bitsets;
@JsonCreator
public DefaultCDSelectorConfiguration(@JsonProperty("modifications_per_lts") DistributionConfig modifications_per_lts,
- @JsonProperty("rows_per_modification") DistributionConfig rows_per_modification,
+ @JsonProperty("rows_per_modification") DistributionConfig operations_per_lts,
Review Comment:
Removed rows_per_modification now and cleaned up modifications_per_lts. Missed this.
##########
harry-core/src/harry/core/Configuration.java:
##########
@@ -885,20 +885,20 @@ public DefaultCDSelectorConfiguration build()
public static class DefaultCDSelectorConfiguration implements CDSelectorConfiguration
{
public final DistributionConfig modifications_per_lts;
Review Comment:
Yes; removed
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414606903
##########
harry-core/src/harry/model/DescriptorSelectorBuilder.java:
##########
@@ -33,7 +33,7 @@ public class DescriptorSelectorBuilder implements Configuration.CDSelectorConfig
{
private Function<SchemaSpec, OpSelectors.ColumnSelector> columnSelectorFactory;
private OpSelectors.OperationSelector operationSelector = DEFAULT_OP_SELECTOR;
- private Distribution numberOfRowsDistribution = new Distribution.ScaledDistribution(2, 30);
+ private Distribution modificationsPerLts = new Distribution.ScaledDistribution(2, 30);
private Distribution numberOfModificationsDistribution = new Distribution.ScaledDistribution(1, 3);
Review Comment:
Good point. Removed!
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412419628
##########
harry-core/src/harry/dsl/HistoryBuilder.java:
##########
@@ -361,32 +350,22 @@ public HistoryBuilder finish()
void addSteps(List<Step> steps)
{
- List<ReplayingVisitor.Batch> batches = new ArrayList<>();
- Counter m = new Counter();
+ List<ReplayingVisitor.Operation> operations = new ArrayList<>();
Counter opId = new Counter();
for (Step step : steps)
{
- batches.add(step.toBatch(pd, lts, m.get(), opId::getAndIncrement));
Review Comment:
Cool, so we no longer need the notion of a modification ID?
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412417625
##########
harry-core/src/harry/core/Configuration.java:
##########
@@ -885,20 +885,20 @@ public DefaultCDSelectorConfiguration build()
public static class DefaultCDSelectorConfiguration implements CDSelectorConfiguration
{
public final DistributionConfig modifications_per_lts;
Review Comment:
Is `modifications_per_lts` here unused now?
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412414582
##########
harry-core/src/harry/core/Configuration.java:
##########
@@ -885,20 +885,20 @@ public DefaultCDSelectorConfiguration build()
public static class DefaultCDSelectorConfiguration implements CDSelectorConfiguration
{
public final DistributionConfig modifications_per_lts;
- public final DistributionConfig rows_per_modification;
+ public final DistributionConfig operations_per_lts;
public final int max_partition_size;
public final Map<OpSelectors.OperationKind, Integer> operation_kind_weights;
public final Map<OpSelectors.OperationKind, long[]> column_mask_bitsets;
@JsonCreator
public DefaultCDSelectorConfiguration(@JsonProperty("modifications_per_lts") DistributionConfig modifications_per_lts,
- @JsonProperty("rows_per_modification") DistributionConfig rows_per_modification,
+ @JsonProperty("rows_per_modification") DistributionConfig operations_per_lts,
Review Comment:
Leaving "rows_per_modification" for back-compat?
--
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] Remove notion of Modification [cassandra-harry]
Posted by "ifesdjeen (via GitHub)" <gi...@apache.org>.
ifesdjeen commented on PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#issuecomment-1839687269
Removed modifications_per_lts and rows_per_modification and introduced operations_per_lts.
I _think_ most of your comments should now be addressed.
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412434836
##########
harry-core/src/harry/model/DescriptorSelectorBuilder.java:
##########
@@ -33,7 +33,7 @@ public class DescriptorSelectorBuilder implements Configuration.CDSelectorConfig
{
private Function<SchemaSpec, OpSelectors.ColumnSelector> columnSelectorFactory;
private OpSelectors.OperationSelector operationSelector = DEFAULT_OP_SELECTOR;
- private Distribution numberOfRowsDistribution = new Distribution.ScaledDistribution(2, 30);
+ private Distribution modificationsPerLts = new Distribution.ScaledDistribution(2, 30);
private Distribution numberOfModificationsDistribution = new Distribution.ScaledDistribution(1, 3);
Review Comment:
`numberOfModificationsDistribution` is unused now?
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412517362
##########
harry-core/test/harry/model/OpSelectorsTest.java:
##########
@@ -73,19 +72,16 @@ public void testRowDataDescriptorSupplier()
for (int lts = 0; lts < RUNS; lts++)
{
long pd = pdSupplier.pd(lts);
- for (int m = 0; m < descriptorSelector.numberOfModifications(lts); m++)
+
+ int opsPerMod = descriptorSelector.operationsPerLts(lts);
Review Comment:
```suggestion
int opsPerLts = descriptorSelector.operationsPerLts(lts);
```
--
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] Remove notion of Modification [cassandra-harry]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1412435393
##########
harry-core/src/harry/model/DescriptorSelectorBuilder.java:
##########
@@ -72,9 +72,9 @@ public DescriptorSelectorBuilder setOperationSelector(OpSelectors.OperationSelec
* In a given modification, we are only able to visit as many rows as there are rows in the partition, so
* we'll always be limited by this.
Review Comment:
nit: Comment needs tweaking?
--
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] Remove notion of Modification [cassandra-harry]
Posted by "aratno (via GitHub)" <gi...@apache.org>.
aratno commented on code in PR #23:
URL: https://github.com/apache/cassandra-harry/pull/23#discussion_r1414746215
##########
harry-core/src/harry/concurrent/Uninterruptibles.java:
##########
@@ -0,0 +1,31 @@
+package harry.concurrent;
+
+import java.util.concurrent.TimeUnit;
+
+public class Uninterruptibles
Review Comment:
Avoiding using Guava?
--
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