You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "netudima (via GitHub)" <gi...@apache.org> on 2025/12/03 10:54:00 UTC
[PR] [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case [cassandra]
netudima opened a new pull request, #4512:
URL: https://github.com/apache/cassandra/pull/4512
Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case
Patch by Dmitry Konstantinov; reviewed by TBD for CASSANDRA-21056
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4512:
URL: https://github.com/apache/cassandra/pull/4512#discussion_r2585673899
##########
src/java/org/apache/cassandra/metrics/ThreadLocalMeter.java:
##########
@@ -92,9 +93,21 @@ public class ThreadLocalMeter extends com.codahale.metrics.Meter implements Mete
maxTicks = m3Ticks;
}
+ private static final ScheduledFuture<?> backgroundTickingJob;
+
+ /**
+ * the method is provided for test purposes only,
+ * to disable background activities and make tests more deterministic
+ */
+ @VisibleForTesting
+ public static void disableBackgroundTicking()
Review Comment:
I prefer the former.
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case [cassandra]
Posted by "netudima (via GitHub)" <gi...@apache.org>.
netudima commented on code in PR #4512:
URL: https://github.com/apache/cassandra/pull/4512#discussion_r2585742868
##########
src/java/org/apache/cassandra/metrics/ThreadLocalMeter.java:
##########
@@ -92,9 +93,21 @@ public class ThreadLocalMeter extends com.codahale.metrics.Meter implements Mete
maxTicks = m3Ticks;
}
+ private static final ScheduledFuture<?> backgroundTickingJob;
+
+ /**
+ * the method is provided for test purposes only,
+ * to disable background activities and make tests more deterministic
+ */
+ @VisibleForTesting
+ public static void disableBackgroundTicking()
Review Comment:
I think package private method is a slightly better option here..
1) it is a bit better encapsulated vs exposing a field for any actions with it
2) the method name self-documents the reason to expose more explicitly
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4512:
URL: https://github.com/apache/cassandra/pull/4512#discussion_r2585747973
##########
src/java/org/apache/cassandra/metrics/ThreadLocalMeter.java:
##########
@@ -92,9 +93,21 @@ public class ThreadLocalMeter extends com.codahale.metrics.Meter implements Mete
maxTicks = m3Ticks;
}
+ private static final ScheduledFuture<?> backgroundTickingJob;
+
+ /**
+ * the method is provided for test purposes only,
+ * to disable background activities and make tests more deterministic
+ */
+ @VisibleForTesting
+ public static void disableBackgroundTicking()
Review Comment:
yes, that's why I prefer former option too :)
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4512:
URL: https://github.com/apache/cassandra/pull/4512#discussion_r2585662674
##########
src/java/org/apache/cassandra/metrics/ThreadLocalMeter.java:
##########
@@ -92,9 +93,21 @@ public class ThreadLocalMeter extends com.codahale.metrics.Meter implements Mete
maxTicks = m3Ticks;
}
+ private static final ScheduledFuture<?> backgroundTickingJob;
+
+ /**
+ * the method is provided for test purposes only,
+ * to disable background activities and make tests more deterministic
+ */
+ @VisibleForTesting
+ public static void disableBackgroundTicking()
Review Comment:
or you could make `backgroundTickingJob` package protected and remove this method
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #4512:
URL: https://github.com/apache/cassandra/pull/4512#discussion_r2585653760
##########
src/java/org/apache/cassandra/metrics/ThreadLocalMeter.java:
##########
@@ -92,9 +93,21 @@ public class ThreadLocalMeter extends com.codahale.metrics.Meter implements Mete
maxTicks = m3Ticks;
}
+ private static final ScheduledFuture<?> backgroundTickingJob;
+
+ /**
+ * the method is provided for test purposes only,
+ * to disable background activities and make tests more deterministic
+ */
+ @VisibleForTesting
+ public static void disableBackgroundTicking()
Review Comment:
can you make this package protected?
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case [cassandra]
Posted by "netudima (via GitHub)" <gi...@apache.org>.
netudima commented on code in PR #4512:
URL: https://github.com/apache/cassandra/pull/4512#discussion_r2585845036
##########
src/java/org/apache/cassandra/metrics/ThreadLocalMeter.java:
##########
@@ -92,9 +93,21 @@ public class ThreadLocalMeter extends com.codahale.metrics.Meter implements Mete
maxTicks = m3Ticks;
}
+ private static final ScheduledFuture<?> backgroundTickingJob;
+
+ /**
+ * the method is provided for test purposes only,
+ * to disable background activities and make tests more deterministic
+ */
+ @VisibleForTesting
+ public static void disableBackgroundTicking()
Review Comment:
done
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case [cassandra]
Posted by "netudima (via GitHub)" <gi...@apache.org>.
netudima closed pull request #4512: [ CASSANDRA-21056][trunk] Fix flaky ThreadLocalMeterTest by disabling background triggering of ThreadLocalMeter#tickAll within the test case
URL: https://github.com/apache/cassandra/pull/4512
--
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-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org