Skip to content

Commit 2581014

Browse files
committed
chore: change the resolution of reply latency to measurement to ns
Also, fix test_reply_count that did not really tested command due to missed await statements. Signed-off-by: Roman Gershman <[email protected]>
1 parent 1e14f80 commit 2581014

File tree

3 files changed

+25
-14
lines changed

3 files changed

+25
-14
lines changed

src/facade/reply_builder.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ void SinkReplyBuilder::Send() {
181181

182182
uint64_t after_ns = util::fb2::ProactorBase::GetMonotonicTimeNs();
183183
reply_stats.send_stats.count++;
184-
reply_stats.send_stats.total_duration += (after_ns - pin.timestamp_ns) / 1'000;
184+
reply_stats.send_stats.total_duration += (after_ns - pin.timestamp_ns);
185185
DVLOG(2) << "Finished writing " << total_size_ << " bytes";
186186
}
187187

src/server/server_family.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,7 +1713,7 @@ void PrintPrometheusMetrics(uint64_t uptime, const Metrics& m, DflyCmd* dfly_cmd
17131713
MetricType::COUNTER, &resp->body());
17141714
{
17151715
AppendMetricWithoutLabels("reply_duration_seconds", "",
1716-
m.facade_stats.reply_stats.send_stats.total_duration * 1e-6,
1716+
m.facade_stats.reply_stats.send_stats.total_duration * 1e-9,
17171717
MetricType::COUNTER, &resp->body());
17181718
AppendMetricWithoutLabels("reply_total", "", m.facade_stats.reply_stats.send_stats.count,
17191719
MetricType::COUNTER, &resp->body());
@@ -3015,8 +3015,6 @@ string ServerFamily::FormatInfoMetrics(const Metrics& m, std::string_view sectio
30153015
append("defrag_attempt_total", m.shard_stats.defrag_attempt_total);
30163016
append("defrag_realloc_total", m.shard_stats.defrag_realloc_total);
30173017
append("defrag_task_invocation_total", m.shard_stats.defrag_task_invocation_total);
3018-
append("reply_count", reply_stats.send_stats.count);
3019-
append("reply_latency_usec", reply_stats.send_stats.total_duration);
30203018

30213019
// Number of connections that are currently blocked on grabbing interpreter.
30223020
append("blocked_on_interpreter", m.coordinator_stats.blocked_on_interpreter);

tests/dragonfly/connection_test.py

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import requests
12
import random
23
import logging
34
import string
@@ -23,6 +24,8 @@
2324
from . import dfly_args
2425
from .instance import DflyInstance, DflyInstanceFactory
2526

27+
from prometheus_client.parser import text_string_to_metric_families
28+
2629
BASE_PORT = 1111
2730

2831

@@ -610,18 +613,29 @@ async def test_keyspace_events_config_set(async_client: aioredis.Redis):
610613
async def test_reply_count(async_client: aioredis.Redis):
611614
"""Make sure reply aggregations reduce reply counts for common cases"""
612615

613-
async def get_reply_count():
614-
return (await async_client.info("STATS"))["reply_count"]
616+
port = int(async_client.get_connection_kwargs().get("port"))
617+
METRICS_URL = f"http://localhost:{port}/metrics"
618+
logging.getLogger("urllib3").setLevel(logging.WARNING)
619+
620+
def get_reply_count():
621+
response = requests.get(METRICS_URL)
622+
response.raise_for_status()
623+
families = text_string_to_metric_families(response.text)
624+
for metric in families:
625+
if metric.name == "dragonfly_reply":
626+
assert metric.type == "counter"
627+
return int(metric.samples[0].value)
628+
return None
615629

616630
async def measure(aw):
617-
before = await get_reply_count()
631+
before = get_reply_count()
618632
await aw
619-
return await get_reply_count() - before - 1
633+
return get_reply_count() - before
620634

621635
await async_client.config_resetstat()
622-
base = await get_reply_count()
623-
info_diff = await get_reply_count() - base
624-
assert info_diff == 1
636+
base = get_reply_count()
637+
info_diff = get_reply_count() - base
638+
assert info_diff == 0 # no commands yet
625639

626640
# Warm client buffer up
627641
await async_client.lpush("warmup", *(i for i in range(500)))
@@ -638,21 +652,20 @@ async def measure(aw):
638652
# Sorted sets
639653
await async_client.zadd("zset-1", mapping={str(i): i for i in range(50)})
640654
assert await measure(async_client.zrange("zset-1", 0, -1, withscores=True)) == 1
641-
642655
# Exec call
643656
e = async_client.pipeline(transaction=True)
644657
for _ in range(100):
645658
e.incr("num-1")
646659

647660
# one - for MULTI-OK, one for the rest. Depends on the squashing efficiency,
648661
# can be either 1 or 2 replies.
649-
assert await measure(e.execute()) <= 2
662+
assert await measure(e.execute()) <= 1
650663

651664
# Just pipeline
652665
p = async_client.pipeline(transaction=False)
653666
for _ in range(100):
654667
p.incr("num-1")
655-
assert await measure(p.execute()) <= 2
668+
assert await measure(p.execute()) <= 1
656669

657670
# Script result
658671
assert await measure(async_client.eval('return {1,2,{3,4},5,6,7,8,"nine"}', 0)) == 1

0 commit comments

Comments
 (0)