URL: https://github.com/freeipa/freeipa/pull/5165
Author: tiran
Title: #5165: Reduce long sleeps in certmonger wait_for_request()
Action: opened
PR body:
"""
## Add helper for poll/sleep loops with timeout
The Sleeper class is a helper that makes poll/sleep loops with timeout
easier to write. It takes care of edge cases and does not oversleep
timeout deadline.
## Faster certmonger wait_for_request()
wait_for_request() now waits 0.5 instead of 5 seconds. This shoves off
15 to 20 seconds from ipa-server-install while marginally increased
load on the system.
Related: https://pagure.io/freeipa/issue/8521
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/5165/head:pr5165
git checkout pr5165
From c69a30f3a6e48616b31ca7aa76aedc0443464487 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Mon, 21 Sep 2020 12:39:52 +0200
Subject: [PATCH 1/2] Add helper for poll/sleep loops with timeout
The Sleeper class is a helper that makes poll/sleep loops with timeout
easier to write. It takes care of edge cases and does not oversleep
timeout deadline.
Related: https://pagure.io/freeipa/issue/8521
Signed-off-by: Christian Heimes <chei...@redhat.com>
---
ipapython/ipautil.py | 63 +++++++++++++++++++++++++
ipatests/test_ipapython/test_ipautil.py | 34 +++++++++++++
2 files changed, 97 insertions(+)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index ebf740be48..71992287c3 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1654,3 +1654,66 @@ def datetime_from_utctimestamp(t, units=1):
raise TypeError(t)
epoch = datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc)
return epoch + datetime.timedelta(seconds=v // units)
+
+
+class Sleeper:
+ """Helper for time.sleep() loop with timeout
+
+ A sleeper object sleeps *sleep* seconds when it is called. Close to its
+ deadline it sleeps shorter to not oversleep the *timeout* deadline. A
+ sleeper object is *True* and returns *True* before it reaches *timeout*
+ deadline. After its deadline a sleeper raises the exception object/class
+ in *raises*. If *raises* is not given, it returns False instead.
+
+ sleep = Sleeper(sleep=1, timeout=60, raises=TimeoutError)
+ while True:
+ do_something()
+ sleep()
+
+ sleep = Sleeper(sleep=0.5, timeout=60)
+ while True:
+ try:
+ do_something
+ except Exception:
+ # sleep duration can be extended
+ sleep(10)
+ else:
+ if not sleep():
+ log.info("timeout")
+ break
+
+ longsleep = Sleeper(sleep=1, timeout=sys.maxsize)
+ """
+ multiplier = 2
+
+ def __init__(self, *, sleep, timeout, raises=None):
+ if timeout <= 0:
+ raise ValueError(f"invalid timeout {timeout}")
+ if sleep < 0.01:
+ raise ValueError(f"sleep duration {sleep} is too short.")
+
+ self.timeout = timeout
+ self.sleep = sleep
+ self.raises = raises
+
+ self.deadline = time.monotonic() + self.timeout
+
+ def __bool__(self):
+ return time.monotonic() < self.deadline
+
+ def __call__(self, extended_sleep=None):
+ now = time.monotonic()
+ if now >= self.deadline:
+ if self.raises is not None:
+ raise self.raises
+ else:
+ return False
+
+ # caller can instruct sleeper to sleep longer
+ dur = self.sleep if extended_sleep is None else extended_sleep
+ # but don't sleep over deadline
+ dur = min(self.deadline - now, dur)
+
+ time.sleep(dur)
+
+ return True
diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py
index 681313ce01..5137d80cce 100644
--- a/ipatests/test_ipapython/test_ipautil.py
+++ b/ipatests/test_ipapython/test_ipautil.py
@@ -28,6 +28,7 @@
import sys
import tempfile
import textwrap
+import time
import pytest
import six
@@ -604,3 +605,36 @@ def test_config_replace_variables(tempdir):
with open(conffile, 'r') as f:
newconf = f.read()
assert newconf == expected
+
+
+def test_sleeper():
+ start = time.monotonic()
+ sleep = ipautil.Sleeper(sleep=0.020, timeout=0.200, raises=TimeoutError)
+ assert sleep
+
+ with pytest.raises(TimeoutError):
+ while True:
+ sleep()
+
+ dur = time.monotonic() - start
+ assert not sleep
+ assert dur >= 0.2
+ # should finish in 0.2, accept longer time in case the system is busy
+ assert dur < 1.
+
+ start = time.monotonic()
+ loops = 0
+ sleep = ipautil.Sleeper(sleep=0.020, timeout=0.200)
+ assert sleep
+
+ while True:
+ if not sleep():
+ break
+ loops += 1
+
+ dur = time.monotonic() - start
+ assert not sleep
+ assert dur >= 0.2
+ assert dur < 1.
+ # should be 10 loops, accept 9 for slow systems
+ assert loops in {9, 10}
From da15baf0644b720afa8307ff80fbe3fe7be5b8a7 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Mon, 21 Sep 2020 12:45:08 +0200
Subject: [PATCH 2/2] Faster certmonger wait_for_request()
wait_for_request() now waits 0.5 instead of 5 seconds. This shoves off
15 to 20 seconds from ipa-server-install while marginally increased
load on the system.
request_and_wait_for_cert() now uses correct certmonger_wait_timeout
instead of http_timeout.
Related: https://pagure.io/freeipa/issue/8521
Signed-off-by: Christian Heimes <chei...@redhat.com>
---
ipalib/install/certmonger.py | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/ipalib/install/certmonger.py b/ipalib/install/certmonger.py
index 9e8d46fc17..a5d410cf83 100644
--- a/ipalib/install/certmonger.py
+++ b/ipalib/install/certmonger.py
@@ -34,6 +34,7 @@
from ipalib import api
from ipalib.constants import CA_DBUS_TIMEOUT
from ipapython.dn import DN
+from ipapython.ipautil import Sleeper
from ipaplatform.paths import paths
from ipaplatform import services
@@ -367,11 +368,15 @@ def request_and_wait_for_cert(
certpath, subject, principal, nickname, passwd_fname, dns, ca,
profile, pre_command, post_command, storage, perms
)
+ # Don't wait longer than resubmit timeout if it is configured
+ certmonger_timeout = api.env.certmonger_wait_timeout
+ if resubmit_timeout and resubmit_timeout < certmonger_timeout:
+ certmonger_timeout = resubmit_timeout
deadline = time.time() + resubmit_timeout
while True: # until success, timeout, or error
try:
- state = wait_for_request(req_id, api.env.http_timeout)
+ state = wait_for_request(req_id, certmonger_timeout)
except RuntimeError as e:
logger.debug("wait_for_request raised %s", e)
state = 'TIMEOUT'
@@ -772,14 +777,20 @@ def check_state(dirs):
def wait_for_request(request_id, timeout=120):
- for _i in range(0, timeout, 5):
- state = get_request_value(request_id, 'status')
- logger.debug("certmonger request is in state %r", state)
- if state in ('CA_REJECTED', 'CA_UNREACHABLE', 'CA_UNCONFIGURED',
- 'NEED_GUIDANCE', 'NEED_CA', 'MONITORING'):
+ sleep = Sleeper(
+ sleep=0.5, # getcert.c:waitfor() uses 125ms
+ timeout=timeout,
+ raises=RuntimeError("request timed out")
+ )
+ last_state = None
+ while True:
+ state = str(get_request_value(request_id, 'status'))
+ if state != last_state:
+ logger.debug("certmonger request is in state %r", state)
+ if state in {'CA_REJECTED', 'CA_UNREACHABLE', 'CA_UNCONFIGURED',
+ 'NEED_GUIDANCE', 'NEED_CA', 'MONITORING'}:
break
- time.sleep(5)
- else:
- raise RuntimeError("request timed out")
+ last_state = state
+ sleep()
return state
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
