#956 Flow control fixes #957
Conversation
|
Push just resets to preferred email. |
|
@bucko909 : could you please check failed tests, pretty please? |
|
And rebase or conflict fix needed too |
It's not really clear what to do here. If someone has altered their config file to remove this line, this patch will alter behaviour of their instance. However, the example config default has been 10000 since first commit, and that's probably a more sensible default value. It might nonetheless be more sensible to just document the discrepancy.
|
Rebased to fix conflicts. Only meaningful change is the conflict, which was in the middle of these changes, and is added as context in the diff: $ (FROM=4e105a0; NOW=d7926bd; diff -u5 <(git diff -U10 $(git merge-base $FROM origin/master)..$FROM) <(git diff -U10 $(git merge-base $NOW origin/master)..$NOW))
--- /dev/fd/63 2024-08-29 15:20:54.847098593 +0100
+++ /dev/fd/62 2024-08-29 15:20:54.851098553 +0100
@@ -131,14 +131,14 @@
# If enabled this setting is used to timeout metric client connection if no
# metrics have been sent in specified time in seconds
#METRIC_CLIENT_IDLE_TIMEOUT = None
diff --git a/lib/carbon/cache.py b/lib/carbon/cache.py
-index cf8b7d1..a7a0c7e 100644
+index 72ccf6c..0304143 100644
--- a/lib/carbon/cache.py
+++ b/lib/carbon/cache.py
-@@ -200,20 +200,27 @@ class _MetricCache(defaultdict):
+@@ -201,20 +201,27 @@ class _MetricCache(defaultdict):
in self.items()]
@property
def watermarks(self):
return [(metric, min(datapoints.keys()), max(datapoints.keys()))
@@ -162,11 +162,11 @@
def _check_available_space(self):
if state.cacheTooFull and self.size < settings.CACHE_SIZE_LOW_WATERMARK:
log.msg("MetricCache below watermark: self.size=%d" % self.size)
events.cacheSpaceAvailable()
-@@ -244,22 +251,26 @@ class _MetricCache(defaultdict):
+@@ -245,22 +252,26 @@ class _MetricCache(defaultdict):
return sorted(datapoint_index.items(), key=by_timestamp)
def store(self, metric, datapoint):
timestamp, value = datapoint
@@ -180,20 +180,20 @@
else:
+ if self.is_nearly_full:
+ # This will disable reading when flow control is enabled
+ log.msg("MetricCache is nearly full: self.size=%d" % self.size)
+ events.cacheFull()
+ if not self[metric]:
+ self.new_metrics.append(metric)
self.size += 1
self[metric][timestamp] = value
if self.strategy:
self.strategy.store(metric)
else:
# Updating a duplicate does not increase the cache size
self[metric][timestamp] = value
-
- _Cache = None
diff --git a/lib/carbon/client.py b/lib/carbon/client.py
index d9ba5a9..6bed7b7 100644
--- a/lib/carbon/client.py
+++ b/lib/carbon/client.py
@@ -28,20 +28,24 @@ try: |
I've used a smaller overage for this setting, as the cache is a much slower process than the relay, so hopefully its size is much higher.
|
I think this should fix the tests -- apologies; I just ran The fixes are squashed into "[#956] allow instances to go 25% over their hard max".
|
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #957 +/- ##
==========================================
- Coverage 50.63% 50.46% -0.18%
==========================================
Files 36 36
Lines 3446 3474 +28
Branches 535 534 -1
==========================================
+ Hits 1745 1753 +8
- Misses 1574 1592 +18
- Partials 127 129 +2 ☔ View full report in Codecov by Sentry. |
See #956.
This is not a complete fix for the aggregator, but does fix the cache, relay and rewriter daemons.
I've tested the cache daemon manually and verified that it only outputs "MetricCache is nearly full" and "MetricCache below watermark" with a sufficiently high cache size. With a small cache size, it will still drop datapoints ("MetricCache is full") with these settings.
The "fix MAX_QUEUE_SIZE to match config file default" commit may be controversial. I'm happy to do whatever with this to get this merged; it just felt wrong when I found it.