Skip to content

Commit

Permalink
Keep track of proposal state, used by reconciler, only in RAM. (#1324)
Browse files Browse the repository at this point in the history
This PR updates the `ReconcilerState` class to use an in-memory dictionary for state storage instead of file operations. This change simplifies initialization and eliminates file system interactions, improving performance and reliability.

The main effect of this change is to make the reconciler retry proposals that formerly failed to be submitted *immediately after restart*, rather than (as is right now) waiting for a configured retry duration.

The configured retry duration is still respected during the execution of the reconciler.
  • Loading branch information
DFINITYManu authored Feb 28, 2025
1 parent 481bd24 commit e4cb0c0
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 45 deletions.
10 changes: 0 additions & 10 deletions release-controller/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,6 @@ bazel run //release-controller:release-controller \
-- --dry-run --verbose
```

Custom path for the reconciler state?


```sh
export RECONCILER_STATE_DIR=/tmp/dryrun/reconciler-state
bazel run //release-controller:release-controller \
--action_env=RECONCILER_STATE_DIR \
-- --dry-run --verbose
```

Typing errors preventing you from running it, because you are editing code and
testing your changes? Add `--output_groups=-mypy` right after `bazel run`.

Expand Down
5 changes: 1 addition & 4 deletions release-controller/reconciler.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
SecurityReleaseNotesRequest,
OrdinaryReleaseNotesRequest,
)
from util import version_name, release_controller_cache_directory
from util import version_name
from watchdog import Watchdog


Expand Down Expand Up @@ -540,9 +540,6 @@ def main() -> None:
else dryrun.DRECli()
)
state = reconciler_state.ReconcilerState(
pathlib.Path(
os.environ.get("RECONCILER_STATE_DIR", release_controller_cache_directory())
),
None if skip_preloading_state else dre.get_election_proposals_by_version,
)
slack_announcer = (
Expand Down
38 changes: 17 additions & 21 deletions release-controller/reconciler_state.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import datetime
import logging
import pathlib
import os
import time
import typing
import dre_cli

Expand Down Expand Up @@ -73,7 +72,6 @@ class ReconcilerState:

def __init__(
self,
path: pathlib.Path,
known_proposal_retriever: typing.Callable[
[], dict[str, dre_cli.ElectionProposal]
]
Expand All @@ -85,8 +83,12 @@ def __init__(
If specified, every proposal mentioned in the known_proposals list will be
recorded to the state database as existing during initialization.
"""
os.makedirs(path, exist_ok=True)
self.path = path
self.state: dict[
str,
tuple[typing.Literal["submitted"], float, int]
| tuple[typing.Literal["malfunction"], float],
] = {}

self._logger = logging.getLogger(self.__class__.__name__)
if known_proposal_retriever:
for replica_version, proposal in known_proposal_retriever().items():
Expand All @@ -99,34 +101,28 @@ def __init__(
)
p.record_submission(proposal["id"])

def _version_path(self, version: str) -> pathlib.Path:
return self.path / version

def version_proposal(
self, version: str
) -> NoProposal | SubmittedProposal | DREMalfunction:
"""Get the proposal ID for the given version. If the version has not been submitted, return None."""
version_file = self._version_path(version)
if not version_file.exists():
res = self.state.get(version)
if res is None:
return NoProposal(version, self)
with open(version_file, encoding="utf8") as vf:
try:
return SubmittedProposal(version, self, int(vf.read()))
except ValueError:
return DREMalfunction(version, self)
elif isinstance(res, tuple) and res[0] == "malfunction":
return DREMalfunction(version, self)
else:
return SubmittedProposal(version, self, res[2])

def _get_proposal_age(self, version: str) -> datetime.datetime:
return datetime.datetime.fromtimestamp(
os.path.getmtime(self._version_path(version))
)
state = self.state[version]
return datetime.datetime.fromtimestamp(state[1])

def _record_malfunction(self, version: str) -> DREMalfunction:
"""Mark a proposal as submitted."""
self._version_path(version).touch()
self.state[version] = ("malfunction", time.time())
return DREMalfunction(version, self)

def _record_proposal_id(self, version: str, proposal_id: int) -> SubmittedProposal:
"""Save the proposal ID for the given version."""
with open(self._version_path(version), "w", encoding="utf8") as f:
f.write(str(proposal_id))
self.state[version] = ("submitted", time.time(), proposal_id)
return SubmittedProposal(version, self, proposal_id)
6 changes: 1 addition & 5 deletions release-controller/tests/test_reconciler.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ class AmnesiacReconcilerState(ReconcilerState):
"""Reconciler state that uses a temporary directory for storage."""

def __init__(self) -> None:
self.tempdir = tempfile.TemporaryDirectory()
super().__init__(pathlib.Path(self.tempdir.name))

def __del__(self) -> None:
self.tempdir.cleanup()
super().__init__()


class MockActiveVersionProvider(object):
Expand Down
5 changes: 0 additions & 5 deletions release-controller/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import hashlib
import math
import os
import pathlib
import sys
import time
import typing
Expand All @@ -28,10 +27,6 @@ def resolve_binary(name: str) -> str:
return name


def release_controller_cache_directory() -> pathlib.Path:
return pathlib.Path.home() / ".cache" / "release-controller"


T = typing.TypeVar("T")


Expand Down

0 comments on commit e4cb0c0

Please sign in to comment.