toptobes commented on code in PR #1943:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1943#discussion_r3625422005
##########
session.go:
##########
@@ -968,26 +969,49 @@ type hostMetrics struct {
TotalLatency int64
}
+// Query attempts could be started and finished out of order when using
speculative execution. Therefore,
+// we issue attempt indexes at query start time (so that Interceptors can use
them) then record those attempts
+// at completion time (so that metrics can correctly express average latency).
type queryMetrics struct {
- totalAttempts int64
- totalLatency int64
+ l sync.RWMutex
Review Comment:
No, atomics would be worse here
- Assuming you're talking about using an `atomic.[Value|Pointer]` over the
whole struct, that would cause more allocations than necessary (where the
necessary amount is zero), especially under contention (unlikely as that may be)
- Assuming you're talking about stores/loads on individual fields, then that
sort of defeats the point of treating `queryMetrics` as one atomic unit, no?
(since you can still have mismatched reads/writes then)
Mutexes under no contention would be as fast as/faster than atomic pointers
under no contention anyways (and again it looks like contention would be highly
unlikely here)
And if you're worried about someone forgetting to access the mutex before
read/write then the actual queryMetrics could always be encapsulated behind
higher order methods/functions which handle the mutex for you
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
