From c7526f943f50e994e94c8287c772c856961833f2 Mon Sep 17 00:00:00 2001 From: David Capwell Date: Mon, 30 Aug 2021 10:18:03 -0700 Subject: [PATCH] Add soft/hard limits to local reads to protect against reading too much data in a single query patch by David Capwell; reviewed by Caleb Rackliffe and Marcus Eriksson for CASSANDRA-16896 --- CHANGES.txt | 1 + NEWS.txt | 11 +- build.xml | 2 + conf/cassandra.yaml | 32 +- ide/idea/workspace.xml | 6 +- .../org/apache/cassandra/config/Config.java | 7 +- .../cassandra/config/DatabaseDescriptor.java | 74 +++- .../cassandra/config/TrackWarnings.java | 108 ++++++ .../apache/cassandra/cql3/QueryOptions.java | 44 +-- .../cql3/selection/ResultSetBuilder.java | 5 + .../cql3/statements/SelectStatement.java | 30 +- .../apache/cassandra/db/ArrayClustering.java | 2 +- .../org/apache/cassandra/db/Clustering.java | 3 +- .../org/apache/cassandra/db/DeletionTime.java | 2 +- .../org/apache/cassandra/db/ReadCommand.java | 206 +++++++--- .../apache/cassandra/db/RowIndexEntry.java | 51 +++ .../LocalReadSizeTooLargeException.java | 29 ++ .../RowIndexEntryTooLargeException.java | 29 ++ .../db/rows/AbstractRangeTombstoneMarker.java | 4 +- .../apache/cassandra/db/rows/ArrayCell.java | 7 + .../apache/cassandra/db/rows/BTreeRow.java | 13 + .../apache/cassandra/db/rows/BufferCell.java | 7 + .../apache/cassandra/db/rows/CellPath.java | 18 +- .../apache/cassandra/db/rows/ColumnData.java | 3 +- .../cassandra/db/rows/ComplexColumnData.java | 10 + .../apache/cassandra/db/rows/NativeCell.java | 7 + .../db/rows/RangeTombstoneBoundMarker.java | 9 + .../db/rows/RangeTombstoneBoundaryMarker.java | 9 + .../db/rows/RangeTombstoneMarker.java | 3 +- .../org/apache/cassandra/db/rows/Row.java | 3 +- .../exceptions/RequestFailureReason.java | 2 +- .../exceptions/TombstoneAbortException.java | 6 +- .../cassandra/io/sstable/IndexInfo.java | 3 +- .../cassandra/metrics/KeyspaceMetrics.java | 26 +- .../cassandra/metrics/TableMetrics.java | 27 +- .../org/apache/cassandra/net/ParamType.java | 7 +- .../cassandra/service/StorageService.java | 94 ++++- .../service/StorageServiceMBean.java | 23 +- .../cassandra/service/reads/ReadCallback.java | 130 ++----- .../trackwarnings/CoordinatorWarnings.java | 198 ++++++++++ .../reads/trackwarnings/WarnAbortCounter.java | 57 +++ .../reads/trackwarnings/WarningContext.java | 83 ++++ .../reads/trackwarnings/WarningsSnapshot.java | 355 +++++++++++++++++ .../cassandra/transport/Dispatcher.java | 16 + .../apache/cassandra/transport/Message.java | 11 + .../transport/messages/BatchMessage.java | 6 + .../transport/messages/ExecuteMessage.java | 6 + .../transport/messages/QueryMessage.java | 6 + .../cassandra/utils/Int64Serializer.java | 49 +++ .../apache/cassandra/utils/ObjectSizes.java | 3 +- test/conf/cassandra.yaml | 14 +- .../distributed/impl/Coordinator.java | 49 ++- .../cassandra/distributed/impl/Instance.java | 43 +- .../test/ClientReadSizeWarningTest.java | 266 ------------- .../AbstractClientSizeWarning.java | 367 ++++++++++++++++++ .../CoordinatorReadSizeWarningTest.java | 86 ++++ .../LocalReadSizeWarningTest.java | 80 ++++ .../RowIndexSizeWarningTest.java | 123 ++++++ .../TombstoneWarningTest.java} | 12 +- .../config/DatabaseDescriptorRefTest.java | 3 + .../config/DatabaseDescriptorTest.java | 151 +++++++ .../config/YamlConfigurationLoaderTest.java | 66 +++- .../org/apache/cassandra/db/CellSpecTest.java | 43 +- .../cassandra/db/ClusteringHeapSizeTest.java | 6 - .../trackwarnings/WarningsSnapshotTest.java | 187 +++++++++ .../cassandra/utils/ObjectSizesTest.java | 50 +++ 66 files changed, 2816 insertions(+), 573 deletions(-) create mode 100644 src/java/org/apache/cassandra/config/TrackWarnings.java create mode 100644 src/java/org/apache/cassandra/db/filter/LocalReadSizeTooLargeException.java create mode 100644 src/java/org/apache/cassandra/db/filter/RowIndexEntryTooLargeException.java create mode 100644 src/java/org/apache/cassandra/service/reads/trackwarnings/CoordinatorWarnings.java create mode 100644 src/java/org/apache/cassandra/service/reads/trackwarnings/WarnAbortCounter.java create mode 100644 src/java/org/apache/cassandra/service/reads/trackwarnings/WarningContext.java create mode 100644 src/java/org/apache/cassandra/service/reads/trackwarnings/WarningsSnapshot.java create mode 100644 src/java/org/apache/cassandra/utils/Int64Serializer.java delete mode 100644 test/distributed/org/apache/cassandra/distributed/test/ClientReadSizeWarningTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/trackwarnings/AbstractClientSizeWarning.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/trackwarnings/CoordinatorReadSizeWarningTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/trackwarnings/LocalReadSizeWarningTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/trackwarnings/RowIndexSizeWarningTest.java rename test/distributed/org/apache/cassandra/distributed/test/{ClientTombstoneWarningTest.java => trackwarnings/TombstoneWarningTest.java} (96%) create mode 100644 test/unit/org/apache/cassandra/service/reads/trackwarnings/WarningsSnapshotTest.java create mode 100644 test/unit/org/apache/cassandra/utils/ObjectSizesTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 2501253fac..388fad1519 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Add soft/hard limits to local reads to protect against reading too much data in a single query (CASSANDRA-16896) * Avoid token cache invalidation for removing a non-member node (CASSANDRA-15290) * Allow configuration of consistency levels on auth operations (CASSANDRA-12988) * Add number of sstables in a compaction to compactionstats output (CASSANDRA-16844) diff --git a/NEWS.txt b/NEWS.txt index 77a3e1bf72..bdd3dd6ffe 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -40,11 +40,14 @@ New features ------------ - Warn/abort thresholds added to read queries notifying clients when these thresholds trigger (by emitting a client warning or aborting the query). This feature is disabled by default, scheduled - to be enabled in 4.2; it is controlled with the configuration client_track_warnings_enabled, + to be enabled in 4.2; it is controlled with the configuration track_warnings.enabled, setting to true will enable this feature. Each check has its own warn/abort thresholds, currently - tombstones (tombstone_warn_threshold, and tombstone_failure_threshold) and coordinator result set - materialized size (client_large_read_warn_threshold_kb, and client_large_read_abort_threshold_kb) - are supported; more checks will be added over time. + tombstones (tombstone_warn_threshold, and tombstone_failure_threshold), coordinator result set + materialized size (track_warnings.coordinator_large_read.warn_threshold_kb, and + track_warnings.coordinator_large_read.abort_threshold_kb), local read materialized heap size + (track_warnings.local_read_size.warn_threshold_kb and track_warnings.local_read_size.abort_threshold_kb), + and RowIndexEntry estimated memory size (track_warnings.row_index_size.warn_threshold_kb and + track_warnings.row_index_size.abort_threshold_kb) are supported; more checks will be added over time. Upgrading --------- diff --git a/build.xml b/build.xml index 32eba7c26a..1dc1c035cd 100644 --- a/build.xml +++ b/build.xml @@ -859,6 +859,7 @@ + @@ -1370,6 +1371,7 @@ + diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index a868a4af0a..4e6db631be 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1459,15 +1459,25 @@ enable_drop_compact_storage: false # - 127.0.0.0/31 # Enables tracking warnings/aborts across all replicas for reporting back to client. -# Scheduled to enable in 4.2 # See: CASSANDRA-16850 -# See: tombstone_warn_threshold, tombstone_failure_threshold, client_large_read_warn_threshold_kb, and client_large_read_abort_threshold_kb -#client_track_warnings_enabled: false - -# When client_track_warnings_enabled: true, this tracks the materialized size of a query on the -# coordinator. If client_large_read_warn_threshold_kb is greater than 0, this will emit a warning -# to clients with details on what query triggered this as well as the size of the result set; if -# client_large_read_abort_threshold_kb is greater than 0, this will abort the query after it -# has exceeded this threshold, returning a read error to the user. -#client_large_read_warn_threshold_kb: 0 -#client_large_read_abort_threshold_kb: 0 +#track_warnings: +# # Scheduled to enable in 4.2 +# enabled: false +# # When track_warnings.enabled: true, this tracks the materialized size of a query on the +# # coordinator. If coordinator_large_read.warn_threshold_kb is greater than 0, this will emit a warning +# # to clients with details on what query triggered this as well as the size of the result set; if +# # coordinator_large_read.abort_threshold_kb is greater than 0, this will abort the query after it +# # has exceeded this threshold, returning a read error to the user. +# coordinator_large_read: +# warn_threshold_kb: 0 +# abort_threshold_kb: 0 +# # When track_warnings.enabled: true, this tracks the size of the local read (as defined by +# # heap size), and will warn/abort based off these thresholds; 0 disables these checks. +# local_read_size: +# warn_threshold_kb: 0 +# abort_threshold_kb: 0 +# # When track_warnings.enabled: true, this tracks the expected memory size of the RowIndexEntry +# # and will warn/abort based off these thresholds; 0 disables these checks. +# row_index_size: +# warn_threshold_kb: 0 +# abort_threshold_kb: 0 diff --git a/ide/idea/workspace.xml b/ide/idea/workspace.xml index 41645f5ce9..73af47fcc8 100644 --- a/ide/idea/workspace.xml +++ b/ide/idea/workspace.xml @@ -143,7 +143,7 @@