From 8ea799508ce5b269be16d897dc3f59c43225882d Mon Sep 17 00:00:00 2001 From: David Guerrero Date: Tue, 2 Oct 2018 10:50:45 +0200 Subject: [PATCH 1/2] Using direct assignment for writing into mmap instead of struct.pack_into Using pack_into can create atomicity problems when writing on cpython, setting the bytes to 0 before writing the value. When having a lot of writes, some reads would return 0 instead of the real value. Signed-off-by: David Guerrero --- prometheus_client/core.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/prometheus_client/core.py b/prometheus_client/core.py index b05bf645..5ef00f53 100644 --- a/prometheus_client/core.py +++ b/prometheus_client/core.py @@ -31,7 +31,7 @@ _INITIAL_MMAP_SIZE = 1 << 20 _pack_integer = struct.Struct(b'i').pack_into -_pack_double = struct.Struct(b'd').pack_into +_pack_double_func = struct.Struct(b'd').pack _unpack_integer = struct.Struct(b'i').unpack_from _unpack_double = struct.Struct(b'd').unpack_from @@ -493,6 +493,10 @@ def get(self): return self._value +def _pack_double(data, pos, value): + data[pos:pos + 8] = _pack_double_func(value) + + class _MmapedDict(object): """A dict of doubles, backed by an mmapped file. From eaf5a4265434fa5605c40e96bd26da2f7f4e6cae Mon Sep 17 00:00:00 2001 From: David Guerrero Date: Tue, 2 Oct 2018 11:19:26 +0200 Subject: [PATCH 2/2] Adding the mmap write fix for integer writes, added a comment to clarify the issue. Signed-off-by: David Guerrero --- prometheus_client/core.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/prometheus_client/core.py b/prometheus_client/core.py index 5ef00f53..29dc29bb 100644 --- a/prometheus_client/core.py +++ b/prometheus_client/core.py @@ -30,7 +30,7 @@ _MINUS_INF = float("-inf") _INITIAL_MMAP_SIZE = 1 << 20 -_pack_integer = struct.Struct(b'i').pack_into +_pack_integer_func = struct.Struct(b'i').pack _pack_double_func = struct.Struct(b'd').pack _unpack_integer = struct.Struct(b'i').unpack_from _unpack_double = struct.Struct(b'd').unpack_from @@ -493,10 +493,17 @@ def get(self): return self._value +# struct.pack_into has atomicity issues because it will temporarily write 0 into +# the mmap, resulting in false reads to 0 when experiencing a lot of writes. +# Using direct assignment solves this issue. def _pack_double(data, pos, value): data[pos:pos + 8] = _pack_double_func(value) +def _pack_integer(data, pos, value): + data[pos:pos + 4] = _pack_integer_func(value) + + class _MmapedDict(object): """A dict of doubles, backed by an mmapped file.