rustyrazorblade commented on code in PR #44:
URL:
https://github.com/apache/cassandra-easy-stress/pull/44#discussion_r2105461739
##########
manual/MANUAL.adoc:
##########
@@ -53,7 +53,7 @@ $ sudo apt install cassandra-easy-stress
==== RPM Packages
-eYou'll need the bintray repo set up on your machine. Create this
`/etc/yum.repos.d/tlp-tools.repo`:
+You'll need the bintray repo set up on your machine. Create this
`/etc/yum.repos.d/tlp-tools.repo`:
Review Comment:
Hmm... we haven't published these in a long time, and definitely not under
TLP. Feel free to remove this line if you want on commit.
##########
build.gradle:
##########
@@ -101,6 +101,13 @@ dependencies {
implementation 'org.apache.commons:commons-math3:3.6.1'
implementation 'org.hdrhistogram:HdrHistogram:2.1.12'
+ implementation("org.agrona:agrona:1.22.0") // can't use the 2.x or 1.23+
line as it requires JDK 17
Review Comment:
Totally fine with me if we bump to JDK 17. I see close to zero value in
using older JDKs especially when 17+ has Shenandoah. I've seen G1GC pauses in
CES mess up the latency stats. We can do it later though.
##########
src/main/kotlin/com/rustyrazorblade/easycassstress/RequestQueue.kt:
##########
@@ -113,8 +95,6 @@ class RequestQueue(
runner.getNextMutation(key)
}
- op.startTime = getTimer(op).time()
Review Comment:
Also OK to do both - but we should expose the total time (including in
queue) in the output.
##########
src/main/kotlin/com/rustyrazorblade/easycassstress/collector/CompositeCollector.kt:
##########
@@ -0,0 +1,23 @@
+package com.rustyrazorblade.easycassstress.collector
+
+import com.datastax.oss.driver.api.core.cql.AsyncResultSet
+import com.rustyrazorblade.easycassstress.Context
+import com.rustyrazorblade.easycassstress.Either
+import com.rustyrazorblade.easycassstress.StressContext
+import com.rustyrazorblade.easycassstress.workloads.Operation
+
+class CompositeCollector(private vararg val collectors: Collector) : Collector
{
Review Comment:
comment please
##########
src/main/kotlin/com/rustyrazorblade/easycassstress/collector/AsyncCollector.kt:
##########
@@ -0,0 +1,78 @@
+package com.rustyrazorblade.easycassstress.collector
+
+import com.datastax.oss.driver.api.core.cql.AsyncResultSet
+import com.rustyrazorblade.easycassstress.Context
+import com.rustyrazorblade.easycassstress.Either
+import com.rustyrazorblade.easycassstress.StressContext
+import com.rustyrazorblade.easycassstress.workloads.Operation
+import io.netty.util.internal.shaded.org.jctools.queues.MpscArrayQueue
+import org.agrona.concurrent.BackoffIdleStrategy
+import org.slf4j.LoggerFactory
+import java.io.Closeable
+import java.io.File
+import java.util.concurrent.atomic.AtomicInteger
+
+abstract class AsyncCollector(fileOrDirectory: File) : Collector {
Review Comment:
Comment the class please
##########
src/main/kotlin/com/rustyrazorblade/easycassstress/RequestQueue.kt:
##########
@@ -113,8 +95,6 @@ class RequestQueue(
runner.getNextMutation(key)
}
- op.startTime = getTimer(op).time()
Review Comment:
We should expose the total time including queue time, otherwise we suffer
from coordinated omission. For example, if it takes 3 seconds from the time
the request is scheduled to the time it's executed, then the request itself
takes .1 sec, we want it to report 3.1 sec total.
##########
src/main/kotlin/com/rustyrazorblade/easycassstress/collector/Collector.kt:
##########
@@ -0,0 +1,20 @@
+package com.rustyrazorblade.easycassstress.collector
+
+import com.datastax.oss.driver.api.core.cql.AsyncResultSet
+import com.rustyrazorblade.easycassstress.Context
+import com.rustyrazorblade.easycassstress.Either
+import com.rustyrazorblade.easycassstress.StressContext
+import com.rustyrazorblade.easycassstress.workloads.Operation
+
+interface Collector {
Review Comment:
Class comment please
##########
src/main/kotlin/com/rustyrazorblade/easycassstress/collector/AsyncCollector.kt:
##########
@@ -0,0 +1,78 @@
+package com.rustyrazorblade.easycassstress.collector
+
+import com.datastax.oss.driver.api.core.cql.AsyncResultSet
+import com.rustyrazorblade.easycassstress.Context
+import com.rustyrazorblade.easycassstress.Either
+import com.rustyrazorblade.easycassstress.StressContext
+import com.rustyrazorblade.easycassstress.workloads.Operation
+import io.netty.util.internal.shaded.org.jctools.queues.MpscArrayQueue
+import org.agrona.concurrent.BackoffIdleStrategy
+import org.slf4j.LoggerFactory
+import java.io.Closeable
+import java.io.File
+import java.util.concurrent.atomic.AtomicInteger
+
+abstract class AsyncCollector(fileOrDirectory: File) : Collector {
+ data class Event(
+ val op: Operation,
+ val result: Either<AsyncResultSet, Throwable>,
+ val startTimeMs: Long,
+ val durationNs: Long
+ )
+
+ interface Writer : Closeable {
+ fun write(event: Event)
+ }
+
+ private val queue =
MpscArrayQueue<Event>(Integer.getInteger("tlp-stress.event_csv_queue_size",
4096))
+ private val writer = createWriter(fileOrDirectory)
+
+ @Volatile
+ private var running = true
+ private val thread = Thread(this::run)
+ private val idleStrategy = BackoffIdleStrategy()
+
+ init {
+ thread.isDaemon = true
+ thread.name = "tlp-stress event raw log collector"
Review Comment:
tlp-stress should be cassandra-easy-stress
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
