Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions tests/test_runtime_sqlalchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,36 @@ def test_pull_and_upsert(self):
for value in row:
self.assertNotIn(value, (None, ""))

def test_observed_cpu_overrides_config_and_gpu_uses_config(self):
redis = _FakeRedis(
{
"future:xyz": {
"id": "xyz",
"request_id": "req2",
"agent": "AgentB",
"created_at": "10.0",
"finished_at": "12.0",
"execution_time": "1.25",
"cpu_resource": "37.5",
"gpu_resource": "256",
},
"request:req2:workflow": "wf2",
}
)

rows = sqlmod.pull_data(redis)
sqlmod.send_data(rows, {"AgentB": {"cpu": 8, "gpu": 0}}, redis)
with sqlmod._get_engine("").connect() as conn:
row = conn.execute(
text(
"SELECT execution_time, cpu_resource, gpu_resource "
"FROM runtime_information WHERE future_id='xyz'"
)
).fetchone()
self.assertEqual(row[0], 2.0)
self.assertEqual(row[1], 37.5)
self.assertEqual(row[2], 0.0)


if __name__ == "__main__":
unittest.main()
12 changes: 11 additions & 1 deletion ventis/controller/local_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,9 @@ def _execute_locally(
self, service, function, args, future_id, origin=None, request_id=None
):
"""Execute a request on the local agent and write the result to Redis."""
wall_start = time.time()
thread_cpu_start = time.thread_time()

# Propagate the request_id context into this worker thread
if request_id:
ventis_context.set_request_id(request_id)
Expand Down Expand Up @@ -461,7 +464,14 @@ def _execute_locally(
if origin and origin != self._my_endpoint:
self._send_result_callback(origin, future_id, f"Execution failed: {e}")

self.redis.hset(f"future:{future_id}", "finished_at", time.time())
wall_end = time.time()
self.redis.hset(f"future:{future_id}", "finished_at", wall_end)

wall_duration = max(wall_end - wall_start, 0.0)
cpu_seconds = max(time.thread_time() - thread_cpu_start, 0.0)
cpu_percent = (cpu_seconds / wall_duration * 100.0) if wall_duration else 0.0

self.redis.hset(f"future:{future_id}", "cpu_resource", cpu_percent)
Comment on lines +467 to +474

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Publish completion metrics atomically.

pull_data() scans any populated future:* hash, while send_data() falls back to configured CPU when cpu_resource is absent. Lines 468 and 474 use separate Redis commands, so ingestion can observe finished_at without the observed CPU and persist the static fallback.

Use one hset_multiple completion write, or make ingestion ignore hashes until finished_at is present and retry partial records.

Proposed fix
-        self.redis.hset(f"future:{future_id}", "finished_at", wall_end)
-
         wall_duration = max(wall_end - wall_start, 0.0)
         cpu_seconds = max(time.thread_time() - thread_cpu_start, 0.0)
         cpu_percent = (cpu_seconds / wall_duration * 100.0) if wall_duration else 0.0

-        self.redis.hset(f"future:{future_id}", "cpu_resource", cpu_percent)
+        self.redis.hset_multiple(
+            f"future:{future_id}",
+            {"finished_at": wall_end, "cpu_resource": cpu_percent},
+        )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ventis/controller/local_controller.py` around lines 467 - 474, Update the
completion writes in the pull_data completion path to publish finished_at and
cpu_resource atomically via one Redis hset_multiple call. Ensure ingestion
cannot observe a populated future hash with finished_at present but without the
calculated CPU metric, while preserving the existing wall-duration and
CPU-percentage calculations.


# ------------------------------------------------------------------ #
# Request forwarding #
Expand Down
8 changes: 5 additions & 3 deletions ventis/controller/utils/sqlalchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def send_data(
redis_client: RedisClient | None = None,
database_url="",
):
"""UPSERT rows and attach allocated cpu/gpu from resources_by_agent."""
"""UPSERT rows with observed CPU and configured GPU resource values."""
if not rows:
return
resources_by_agent = resources_by_agent or {}
Expand All @@ -77,6 +77,8 @@ def send_data(
)
start = float(raw.get("created_at") or 0)
end = float(raw.get("finished_at") or time.time())
cpu_resource = float(raw.get("cpu_resource") or res.get("cpu", 0))
gpu_resource = float(res.get("gpu", 0))
Comment on lines +80 to +81

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Normalize resource values before persistence.

float() accepts negative observed CPU and configured GPU values, allowing malformed Redis/configuration data to write negative resource metrics. Clamp or reject both values before the UPSERT.

Proposed fix
-            cpu_resource = float(raw.get("cpu_resource") or res.get("cpu", 0))
-            gpu_resource = float(res.get("gpu", 0))
+            cpu_resource = max(
+                float(raw.get("cpu_resource") or res.get("cpu", 0)), 0.0
+            )
+            gpu_resource = max(float(res.get("gpu", 0)), 0.0)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cpu_resource = float(raw.get("cpu_resource") or res.get("cpu", 0))
gpu_resource = float(res.get("gpu", 0))
cpu_resource = max(
float(raw.get("cpu_resource") or res.get("cpu", 0)), 0.0
)
gpu_resource = max(float(res.get("gpu", 0)), 0.0)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ventis/controller/utils/sqlalchemy.py` around lines 80 - 81, Normalize
cpu_resource and gpu_resource immediately after conversion and before the
UPSERT, ensuring both observed CPU and configured GPU values cannot persist as
negative metrics. Use the existing resource-processing flow to clamp them to
zero or reject invalid values consistently, while preserving valid non-negative
values.


conn.execute(
_UPSERT,
Expand All @@ -86,8 +88,8 @@ def send_data(
"workflow": workflow,
"agent": agent,
"execution_time": end - start,
Comment thread
Saaketh0 marked this conversation as resolved.
"cpu_resource": float(res.get("cpu", 0)),
"gpu_resource": float(res.get("gpu", 0)),
"cpu_resource": cpu_resource,
"gpu_resource": gpu_resource,
"created_at": str(start),
"updated_at": str(end),
},
Expand Down