You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/09/13 18:26:00 UTC
[GitHub] [cassandra] josh-mckenzie opened a new pull request, #1855: Cassandra 17868 - Allow disabling hotness persistence for high sstable counts
josh-mckenzie opened a new pull request, #1855:
URL: https://github.com/apache/cassandra/pull/1855
The persisting of the sstables hotness when there are 10s of thousands of sstables can have issues keeping up, and the rate limiter is hard coded. Another option may be nice to just completely disable the feature.
When sstables super backed up (from repairs) the hotness tracking tends to cause the STCS in L0 to make poor decisions, always grabbing the large sstables and skipping the tiny sstables that would benefit more.
--
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
[GitHub] [cassandra] josh-mckenzie closed pull request #1855: Cassandra 17868 - Allow disabling hotness persistence for high sstable counts
Posted by GitBox <gi...@apache.org>.
josh-mckenzie closed pull request #1855: Cassandra 17868 - Allow disabling hotness persistence for high sstable counts
URL: https://github.com/apache/cassandra/pull/1855
--
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
[GitHub] [cassandra] maedhroz commented on a diff in pull request #1855: Cassandra 17868 - Allow disabling hotness persistence for high sstable counts
Posted by GitBox <gi...@apache.org>.
maedhroz commented on code in PR #1855:
URL: https://github.com/apache/cassandra/pull/1855#discussion_r972409542
##########
test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java:
##########
@@ -247,6 +253,23 @@ public void testReadRateTracking()
Util.getAll(Util.cmd(store, key).includeRow("0").build());
assertEquals(2, sstable.getReadMeter().count());
+
+ // With persistence enabled, we should be able to retrieve the state of the meter.
+ sstable.maybePersistSSTableReadMeter();
+
+ UntypedResultSet meter = SystemKeyspace.readSSTableActivity(store.keyspace.getName(), store.name, sstable.descriptor.id);
+ // So this is the only issue atm; seems like it's empty
Review Comment:
nit: What does this comment mean? Looks like the assertion verifies that it's _not_ empty?
--
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
[GitHub] [cassandra] maedhroz commented on a diff in pull request #1855: Cassandra 17868 - Allow disabling hotness persistence for high sstable counts
Posted by GitBox <gi...@apache.org>.
maedhroz commented on code in PR #1855:
URL: https://github.com/apache/cassandra/pull/1855#discussion_r972409542
##########
test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java:
##########
@@ -247,6 +253,23 @@ public void testReadRateTracking()
Util.getAll(Util.cmd(store, key).includeRow("0").build());
assertEquals(2, sstable.getReadMeter().count());
+
+ // With persistence enabled, we should be able to retrieve the state of the meter.
+ sstable.maybePersistSSTableReadMeter();
+
+ UntypedResultSet meter = SystemKeyspace.readSSTableActivity(store.keyspace.getName(), store.name, sstable.descriptor.id);
+ // So this is the only issue atm; seems like it's empty
Review Comment:
nit: What does this comment mean? Looks like the assertion verifies that is't _not_ empty?
--
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
[GitHub] [cassandra] josh-mckenzie commented on a diff in pull request #1855: Cassandra 17868 - Allow disabling hotness persistence for high sstable counts
Posted by GitBox <gi...@apache.org>.
josh-mckenzie commented on code in PR #1855:
URL: https://github.com/apache/cassandra/pull/1855#discussion_r974484108
##########
test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java:
##########
@@ -247,6 +253,23 @@ public void testReadRateTracking()
Util.getAll(Util.cmd(store, key).includeRow("0").build());
assertEquals(2, sstable.getReadMeter().count());
+
+ // With persistence enabled, we should be able to retrieve the state of the meter.
+ sstable.maybePersistSSTableReadMeter();
+
+ UntypedResultSet meter = SystemKeyspace.readSSTableActivity(store.keyspace.getName(), store.name, sstable.descriptor.id);
+ // So this is the only issue atm; seems like it's empty
Review Comment:
Vestigial from interim debugging; removed.
--
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
[GitHub] [cassandra] josh-mckenzie commented on a diff in pull request #1855: Cassandra 17868 - Allow disabling hotness persistence for high sstable counts
Posted by GitBox <gi...@apache.org>.
josh-mckenzie commented on code in PR #1855:
URL: https://github.com/apache/cassandra/pull/1855#discussion_r974485310
##########
test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java:
##########
@@ -247,6 +253,23 @@ public void testReadRateTracking()
Util.getAll(Util.cmd(store, key).includeRow("0").build());
assertEquals(2, sstable.getReadMeter().count());
+
+ // With persistence enabled, we should be able to retrieve the state of the meter.
+ sstable.maybePersistSSTableReadMeter();
+
+ UntypedResultSet meter = SystemKeyspace.readSSTableActivity(store.keyspace.getName(), store.name, sstable.descriptor.id);
+ // So this is the only issue atm; seems like it's empty
+ assertFalse(meter.isEmpty());
+
+ Util.getAll(Util.cmd(store, key).includeRow("0").build());
+ assertEquals(3, sstable.getReadMeter().count());
+
+ // After cleaning existing state and disabling persistence, there should be no meter state to read.
+ SystemKeyspace.clearSSTableReadMeter(store.keyspace.getName(), store.name, sstable.descriptor.id);
+ DatabaseDescriptor.setSStableReadRatePersistenceEnabled(false);
Review Comment:
Good catch; I'm going to try/finally wrap it on commit just to make sure it reverts to whatever it entered with. Better safe than sorry.
--
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
[GitHub] [cassandra] maedhroz commented on a diff in pull request #1855: Cassandra 17868 - Allow disabling hotness persistence for high sstable counts
Posted by GitBox <gi...@apache.org>.
maedhroz commented on code in PR #1855:
URL: https://github.com/apache/cassandra/pull/1855#discussion_r972410813
##########
test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java:
##########
@@ -247,6 +253,23 @@ public void testReadRateTracking()
Util.getAll(Util.cmd(store, key).includeRow("0").build());
assertEquals(2, sstable.getReadMeter().count());
+
+ // With persistence enabled, we should be able to retrieve the state of the meter.
+ sstable.maybePersistSSTableReadMeter();
+
+ UntypedResultSet meter = SystemKeyspace.readSSTableActivity(store.keyspace.getName(), store.name, sstable.descriptor.id);
+ // So this is the only issue atm; seems like it's empty
+ assertFalse(meter.isEmpty());
+
+ Util.getAll(Util.cmd(store, key).includeRow("0").build());
+ assertEquals(3, sstable.getReadMeter().count());
+
+ // After cleaning existing state and disabling persistence, there should be no meter state to read.
+ SystemKeyspace.clearSSTableReadMeter(store.keyspace.getName(), store.name, sstable.descriptor.id);
+ DatabaseDescriptor.setSStableReadRatePersistenceEnabled(false);
Review Comment:
nit: Any risk to any of the other tests if we don't switch back to the original setting?
--
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