bkonold commented on a change in pull request #1158: SAMZA-2324: Adding KV
store metrics for rocksdb
URL: https://github.com/apache/samza/pull/1158#discussion_r327903055
##########
File path:
samza-kv/src/main/scala/org/apache/samza/storage/kv/SerializedKeyValueStore.scala
##########
@@ -55,23 +55,27 @@ class SerializedKeyValueStore[K, V](
}
def put(key: K, value: V) {
- metrics.puts.inc
val keyBytes = toBytesOrNull(key, keySerde)
val valBytes = toBytesOrNull(value, msgSerde)
store.put(keyBytes, valBytes)
+ metrics.puts.inc
+
metrics.maxRecordSizeBytes.set(Math.max(metrics.maxRecordSizeBytes.getValue,
valBytes.length))
Review comment:
Okay, you're right that it shouldn't be the responsibility of Gauge to warn
about read then write thread safety. Easiest thing to do might be to replace
with
`while(!gauge.compareAndSet(gauge.getValue,valSizeBytes);` or simply to use
a lock.
Histogram feels like it is overkill since the distribution of record size
may not be interesting... Max is interesting because it tells us how close we
are to the kafka limit. Record size otherwise (median, 75th, 99th, etc) is
interesting but will it be useful to customers in a meaningful way?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
