Skip to content

Conversation

@fbarreir
Copy link
Contributor

No description provided.

@fbarreir fbarreir requested a review from Copilot October 16, 2025 09:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Leverages normalized CPU model strings and scraped CPU benchmarks to improve CPU time estimation for scouting jobs and related workflows.

  • Introduces CPU model normalization and hostname cleaning in CoreUtils and uses them across modules.
  • Stores normalized CPU model and CPU benchmark types; uses host- and site-specific benchmarks to determine core power with sensible fallbacks.
  • Minor fixes to API logging and retry guards.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pandaserver/taskbuffer/db_proxy_mods/worker_module.py Centralizes hostname cleaning, adds cpu_model_normalized handling, and introduces a DB lookup for CPU benchmarks by host.
pandaserver/taskbuffer/db_proxy_mods/task_utils_module.py Extends job queries to fetch modificationhost and uses CPU benchmarks for core power calculation with fallbacks.
pandaserver/taskbuffer/db_proxy_mods/task_complex_module.py Adds a None-guard to task attempt limit logic.
pandaserver/taskbuffer/TaskBuffer.py Threads cpu_model_normalized through TaskBuffer.update_worker_node.
pandaserver/srvcore/CoreUtils.py Adds normalize_cpu_model and clean_host_name helpers used by multiple modules.
pandaserver/daemons/scripts/hs_scrapers.py Stores normalized CPU type when inserting into cpu_benchmarks.
pandaserver/api/v1/pilot_api.py Normalizes CPU model before updating worker nodes; fixes GPU update log label.
pandajedi/jedicore/MsgWrapper.py Improves error message content for uploadLog failures.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +704 to +709
benchmarks = []
atlas_site = "NO_SITE" # in case of no match
if site_mapper:
atlas_site = site_mapper.getSite(computingSite).pandasite
benchmarks = self.get_cpu_benchmarks_by_host(atlas_site, modificationhost) or []

Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

self.get_cpu_benchmarks_by_host is not defined in this module; the helper was added to WorkerModule, so this call will raise AttributeError unless the method is injected into this class at runtime. Move get_cpu_benchmarks_by_host to a shared base (e.g., BaseModule) so it is available here, or import and call the WorkerModule implementation explicitly.

Copilot uses AI. Check for mistakes.
Comment on lines +1811 to +1815
"SELECT cb.site, score_per_core FROM atlas_panda.worker_node wn, atlas_panda.cpu_benchmarks cb "
"WHERE wn.site = :site "
"AND wn.host_name = :host_name "
"AND cb.cpu_type_normalized = wn.cpu_model_normalized "
"AND wn.threads_per_core = cb.smt_enabled + 1"
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

[nitpick] Use explicit JOIN syntax for clarity and maintainability. For example: SELECT cb.site, cb.score_per_core FROM atlas_panda.worker_node wn JOIN atlas_panda.cpu_benchmarks cb ON cb.cpu_type_normalized = wn.cpu_model_normalized AND wn.threads_per_core = cb.smt_enabled + 1 WHERE wn.site = :site AND wn.host_name = :host_name.

Suggested change
"SELECT cb.site, score_per_core FROM atlas_panda.worker_node wn, atlas_panda.cpu_benchmarks cb "
"WHERE wn.site = :site "
"AND wn.host_name = :host_name "
"AND cb.cpu_type_normalized = wn.cpu_model_normalized "
"AND wn.threads_per_core = cb.smt_enabled + 1"
"SELECT cb.site, score_per_core FROM atlas_panda.worker_node wn "
"JOIN atlas_panda.cpu_benchmarks cb "
"ON cb.cpu_type_normalized = wn.cpu_model_normalized "
"AND wn.threads_per_core = cb.smt_enabled + 1 "
"WHERE wn.site = :site "
"AND wn.host_name = :host_name"

Copilot uses AI. Check for mistakes.
@fbarreir fbarreir merged commit 11453d6 into master Oct 16, 2025
1 check passed
@fbarreir fbarreir deleted the scouting_hs branch October 23, 2025 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants