All the fixes #339
All the fixes #339
Conversation
alexsnaps
force-pushed
the
fix_partion_flush
branch
from
4f32165 to
922bf2a
Compare
alexsnaps
force-pushed
the
fix_partion_flush
branch
from
922bf2a to
45b191e
Compare
| @@ -215,6 +215,14 @@ impl AsyncRedisStorage { | |||
| .clone() | |||
| .incr::<&str, i32, u64>("LIMITADOR_LIVE_CHECK", 1) | |||
| .await | |||
| .map_err(|err| { | |||
There was a problem hiding this comment.
This is_alive(&self) isn't used anymore afaict... but there is the .then wrt liveness_latency... So... what shall we do?
There was a problem hiding this comment.
I haven't found a #[allow(dead_code)] that might affect this (not complaining), also, method is_alive is not found in any other part of the project than here (method definition)
There was a problem hiding this comment.
@adam-cattermole what you recon we do? 🔥 the metric? Call it from a dedicated Future?
There was a problem hiding this comment.
I reckon we just burn the metric/method. We can add this back if needed but for now we could just use the datastore_latency from the batcher calls to redis (I know even this metric isn't perfect due to size of batch etc). If we decide we want this down the line it shouldn't be too hard to add it back
There was a problem hiding this comment.
Everything looks good to me, with the exception of cargo not complaining for the unused method is_alive. Maybe @adam-cattermole would have a saying on this?
If you test this in
INFOlevel log as such:And then start the driver, you'd get a first log for the priority flush:
Then further for the per second flushes:
Then shut redis down:
No more logs for the duration of the partition, restart redis, and...