fix: lease-acquisition loop can fail with temporary grpc issues by mangelajo · Pull Request #761 · jumpstarter-dev/jumpstarter-python

mangelajo · 2025-12-01T15:03:46Z

a lease acquisition loop can fail in the following way very easily if a router in openshift/k8s is restarted during the lease get operation.

this retry mechanism avoids this issue.

2025-11-28T15:22:39.557321889Z for lease: There are 9 approved
2025-11-28T15:22:39.557321889Z exporters, (i.e. ) but all of them are
2025-11-28T15:22:39.557321889Z already leased (7:30:58)
2025-11-28T15:22:44.577566967Z [11/28/25 15:22:44] INFO INFO:jumpstarter.client.lease:Waiting lease.py:355
2025-11-28T15:22:44.577566967Z for lease: There are 9 approved
2025-11-28T15:22:44.577566967Z exporters, (i.e. ) but all of them are
2025-11-28T15:22:44.577566967Z already leased (7:31:03)
2025-11-28T15:22:56.436248186Z Error: grpc error: Socket closed

Summary by CodeRabbit

Release Notes

Bug Fixes
- Enhanced lease operations with automatic retry logic and exponential backoff for improved connection handling during transient failures.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

a lease acquisition loop can fail in the following way very easily if a router in openshift/k8s is restarted during the lease get operation. this retry mechanism avoids this issue. ``` 2025-11-28T15:22:39.557321889Z for lease: There are 9 approved 2025-11-28T15:22:39.557321889Z exporters, (i.e. ) but all of them are 2025-11-28T15:22:39.557321889Z already leased (7:30:58) 2025-11-28T15:22:44.577566967Z [11/28/25 15:22:44] INFO INFO:jumpstarter.client.lease:Waiting lease.py:355 2025-11-28T15:22:44.577566967Z for lease: There are 9 approved 2025-11-28T15:22:44.577566967Z exporters, (i.e. ) but all of them are 2025-11-28T15:22:44.577566967Z already leased (7:31:03) 2025-11-28T15:22:56.436248186Z Error: grpc error: Socket closed ```

netlify · 2025-12-01T15:03:53Z

✅ Deploy Preview for jumpstarter-docs ready!

Name	Link
🔨 Latest commit	`78ad580`
🔍 Latest deploy log	https://app.netlify.com/projects/jumpstarter-docs/deploys/692dae55e8eb4a000853fe1d
😎 Deploy Preview	https://deploy-preview-761--jumpstarter-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

coderabbitai · 2025-12-01T15:03:56Z

Walkthrough

The changes introduce a retry mechanism with exponential backoff to the lease acquisition process in the Jumpstarter client. A new private method _get_with_retry() wraps lease retrieval operations and automatically retries on connection errors, while _acquire() is updated to use this resilient method during polling.

Changes

Cohort / File(s)	Summary
Lease acquisition resilience `packages/jumpstarter/jumpstarter/client/lease.py`	Added tenacity import; introduced `_get_with_retry()` private method with retry decorator and error logging to handle `GetLease` operations with exponential backoff on `ConnectionError`; refactored `_acquire()` to call `_get_with_retry()` instead of direct `get()` calls.
Dependency management `packages/jumpstarter/pyproject.toml`	Added tenacity >=8.2.0 to runtime dependencies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Verify the tenacity retry configuration (decorator parameters, backoff strategy, exception types) is correct for the use case
Confirm that replacing get() with _get_with_retry() in the polling loop maintains intended behavior and doesn't mask unexpected failures
Validate that the new dependency version is compatible with existing requirements

Poem

🐰 With tenacity's gentle hand,
Lost leases now firmly stand,
Through errors we exponentially hop,
Each retry ensures we don't stop,
A rabbit's resilience—hurrah! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main change: adding retry logic to handle temporary gRPC issues in the lease-acquisition loop.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/jumpstarter/jumpstarter/client/lease.py (1)
81-98: Consider using tenacity's built-in logging to reduce noise.

The current implementation logs an error on every retry attempt (line 96), which could be noisy during transient network issues. Tenacity provides a cleaner pattern using the before_sleep callback.

Apply this diff to use tenacity's built-in logging mechanism:
-from tenacity import retry, retry_if_exception_type, wait_exponential_jitter
+from tenacity import retry, retry_if_exception_type, wait_exponential_jitter, before_sleep_log

 @retry(
     wait=wait_exponential_jitter(initial=1, max=120, jitter=1),
     retry=retry_if_exception_type(ConnectionError),
+    before_sleep=before_sleep_log(logger, logging.WARNING),
     reraise=True,
 )
 async def _get_with_retry(self):
     """Get lease with exponential backoff retry on ConnectionError.
     
     Retries with exponential backoff and jitter indefinitely when ConnectionError occurs.
     The wait time between retries is capped at 2 minutes (120 seconds).
     Jitter helps prevent thundering herd problems when multiple clients retry simultaneously.
     """
-    try:
-        return await self.get()
-    except ConnectionError as e:
-        logger.error("Error while getting lease %s: %s", self.name, e)
-        raise
+    return await self.get()
This approach logs warnings before each retry sleep instead of errors on every attempt, providing clearer visibility into retry behavior without excessive noise.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between afaf058 and 78ad580.

📒 Files selected for processing (2)

packages/jumpstarter/jumpstarter/client/lease.py (3 hunks)
packages/jumpstarter/pyproject.toml (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

packages/*/pyproject.toml

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Each package's pyproject.toml must include project metadata with Apache-2.0 license only

Files:

packages/jumpstarter/pyproject.toml

**/*.py

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Ruff should be used for code formatting and linting, excluding jumpstarter-protocol package

Files:

packages/jumpstarter/jumpstarter/client/lease.py

🧠 Learnings (8)

📓 Common learnings

Learnt from: michalskrivanek
Repo: jumpstarter-dev/jumpstarter PR: 704
File: packages/jumpstarter/jumpstarter/client/grpc.py:100-107
Timestamp: 2025-10-14T17:43:07.788Z
Learning: In the Jumpstarter client lease model (packages/jumpstarter/jumpstarter/client/grpc.py), `effective_duration` represents the elapsed time for an active lease so far, not the total duration. To calculate expected release time, use `effective_begin_time + duration` (where `duration` is the configured/requested duration), not `effective_begin_time + effective_duration`.

Learnt from: michalskrivanek
Repo: jumpstarter-dev/jumpstarter PR: 589
File: packages/jumpstarter/jumpstarter/exporter/exporter.py:147-155
Timestamp: 2025-08-14T13:11:35.034Z
Learning: In the jumpstarter fork-based architecture, when the exporter's serve() method exits (e.g., due to lease changes), the child process terminates and the parent process automatically restarts it, eliminating concerns about orphaned tasks since the entire process is restarted.