sahildevgon commented on code in PR #22454:
URL: https://github.com/apache/kafka/pull/22454#discussion_r3411479906
##########
server/src/main/java/org/apache/kafka/server/metrics/LinuxIoMetricsCollector.java:
##########
@@ -79,32 +141,51 @@ public long writeBytes() {
* read_bytes: 0
* write_bytes: 0
* cancelled_write_bytes: 0
+ *
+ * <p>Caller must hold the monitor of this instance. {@code lastUpdateMs}
is written last so
+ * that any thread observing the new timestamp via a volatile read also
observes all updated
+ * cached fields.
*/
private boolean updateValues(long now) {
- synchronized (this) {
- try {
- cachedReadBytes = -1L;
- cachedWriteBytes = -1L;
- List<String> lines = Files.readAllLines(path,
StandardCharsets.UTF_8);
- for (String line : lines) {
- if (line.startsWith(READ_BYTES_PREFIX)) {
- cachedReadBytes =
Long.parseLong(line.substring(READ_BYTES_PREFIX.length()));
- } else if (line.startsWith(WRITE_BYTES_PREFIX)) {
- cachedWriteBytes =
Long.parseLong(line.substring(WRITE_BYTES_PREFIX.length()));
- }
+ try {
+ cachedReadBytes = -1L;
Review Comment:
Hi @chia7712 , makes sense, pushed a patch where updateValues now parses
into local variables and only publishes to the volatile cached fields once
parsing succeeds, with lastUpdateMs written last so the
visibility ordering across all 7 fields plus the timestamp is consistent.
Lock-free readers no longer see the transient -1L state.
One small behavior change , on parse failure (catch block), I now leave the
cached fields untouched and return false, instead of setting them all to -1L.
This way a transient /proc/self/io read failure doesn't make every JMX
dashboard suddenly read -1L - readers retain last-known-good values. Happy to
revert to the old "publish -1L on failure" semantic if you'd rather keep it
strictly equivalent to the previous code.
--
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]
