dcapwell commented on code in PR #4238:
URL: https://github.com/apache/cassandra/pull/4238#discussion_r2224005592
##########
src/java/org/apache/cassandra/metrics/AccordMetrics.java:
##########
@@ -212,112 +255,128 @@ else if (txnId.isSomeRead())
}
@Override
- public void onStable(Command cmd)
+ public void onStable(SafeCommandStore safeStore, Command cmd)
{
+ Tracing.trace("Stable {} on {}", cmd.txnId(),
safeStore.commandStore());
long now = AccordTimeService.nowMicros();
AccordMetrics metrics = forTransaction(cmd.txnId());
if (metrics != null)
{
long trxTimestamp = cmd.txnId().hlc();
- metrics.stableLatency.update(now - trxTimestamp,
TimeUnit.MICROSECONDS);
+ metrics.replicaStableLatency.update(now - trxTimestamp,
TimeUnit.MICROSECONDS);
}
}
@Override
- public void onPreApplied(Command cmd)
+ public void onPreApplied(SafeCommandStore safeStore, Command cmd)
{
+ Tracing.trace("Preapplied {} on {}", cmd.txnId(),
safeStore.commandStore());
long now = AccordTimeService.nowMicros();
AccordMetrics metrics = forTransaction(cmd.txnId());
if (metrics != null)
{
Timestamp trxTimestamp = cmd.txnId();
- metrics.preapplyLatency.update(now - trxTimestamp.hlc(),
TimeUnit.MICROSECONDS);
+ metrics.replicaPreapplyLatency.update(now -
trxTimestamp.hlc(), TimeUnit.MICROSECONDS);
PartialDeps deps = cmd.partialDeps();
- metrics.localDependencies.update(deps != null ?
deps.txnIdCount() : 0);
+ metrics.replicaDependencies.update(deps != null ?
deps.txnIdCount() : 0);
}
}
@Override
- public void onApplied(Command cmd, long applyStartTimestamp)
+ public void onApplied(SafeCommandStore safeStore, Command cmd, long t0)
Review Comment:
can we rename back to `applyStartTimestamp` so its a more informative name?
##########
src/java/org/apache/cassandra/metrics/AccordMetrics.java:
##########
@@ -212,112 +255,128 @@ else if (txnId.isSomeRead())
}
@Override
- public void onStable(Command cmd)
+ public void onStable(SafeCommandStore safeStore, Command cmd)
{
+ Tracing.trace("Stable {} on {}", cmd.txnId(),
safeStore.commandStore());
long now = AccordTimeService.nowMicros();
AccordMetrics metrics = forTransaction(cmd.txnId());
if (metrics != null)
{
long trxTimestamp = cmd.txnId().hlc();
- metrics.stableLatency.update(now - trxTimestamp,
TimeUnit.MICROSECONDS);
+ metrics.replicaStableLatency.update(now - trxTimestamp,
TimeUnit.MICROSECONDS);
}
}
@Override
- public void onPreApplied(Command cmd)
+ public void onPreApplied(SafeCommandStore safeStore, Command cmd)
{
+ Tracing.trace("Preapplied {} on {}", cmd.txnId(),
safeStore.commandStore());
long now = AccordTimeService.nowMicros();
AccordMetrics metrics = forTransaction(cmd.txnId());
if (metrics != null)
{
Timestamp trxTimestamp = cmd.txnId();
- metrics.preapplyLatency.update(now - trxTimestamp.hlc(),
TimeUnit.MICROSECONDS);
+ metrics.replicaPreapplyLatency.update(now -
trxTimestamp.hlc(), TimeUnit.MICROSECONDS);
PartialDeps deps = cmd.partialDeps();
- metrics.localDependencies.update(deps != null ?
deps.txnIdCount() : 0);
+ metrics.replicaDependencies.update(deps != null ?
deps.txnIdCount() : 0);
}
}
@Override
- public void onApplied(Command cmd, long applyStartTimestamp)
+ public void onApplied(SafeCommandStore safeStore, Command cmd, long t0)
Review Comment:
I see that `t0` is used in the accord side but similar issue, the name
doesn't tell you anything about what it is
```
long t0 = safeStore.node().elapsed(MICROSECONDS);
...
return command.writes().apply(safeStore, executes, command.partialTxn())
```
its `-1` if not applied or the timestamp before calling `writes().apply`
##########
src/java/org/apache/cassandra/service/accord/AccordCommandStore.java:
##########
@@ -572,4 +576,29 @@ static SafeRedundantBefore max(SafeRedundantBefore a,
SafeRedundantBefore b)
return a.ticket >= b.ticket ? a : b;
}
}
+
+ private @Nullable TableMetadata tableMetadata()
+ {
+ TableMetadataRef metadataRef = this.metadata;
+ if (metadataRef != null)
+ return metadataRef.get();
+
+ TableMetadata metadata = Schema.instance.getTableMetadata(tableId);
+ if (metadata == null)
+ return null;
+ this.metadata = metadata.ref;
+ return metadata;
+ }
+
+ @Override
+ public String toString()
+ {
+ TableMetadata metadata = tableMetadata();
+ StringBuilder sb = new StringBuilder("[");
+ if (metadata != null)
+
sb.append(metadata.keyspace).append('.').append(metadata.name).append('|');
Review Comment:
```suggestion
sb.append(metadata).append('|');
```
`TableMetadata.toString` does this, but its valid CQL so the keyspace/name
might be quoted if its needed
##########
src/java/org/apache/cassandra/service/accord/AccordExecutor.java:
##########
@@ -803,6 +806,7 @@ protected Task()
static abstract class SubmittableTask extends Task
{
+ final WithResources locals = ExecutorLocals.propagate();
Review Comment:
is this to get tracing to work? Think this answers a comment i have above =)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
