Skip to content

feat: get the PilotInfo in one query from PilotAgentsDB - #8779

Merged
fstagni merged 4 commits into
DIRACGrid:integrationfrom
chaen:v9.1_feat_pilot_in_query
Oct 28, 2026
Merged

fstagni merged 4 commits into
DIRACGrid:integrationfrom
chaen:v9.1_feat_pilot_in_query

Conversation

@chaen

@chaen chaen commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

BEGINRELEASENOTES

*WMS

CHANGE: select pilot info in a single query instead of a large IN list and 2 queries

ENDRELEASENOTES

@chaen
chaen force-pushed the v9.1_feat_pilot_in_query branch 2 times, most recently from c4d527e to 1a65ad7 Compare September 8, 2026 15:49
@aldbr

aldbr commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

I am not fully sure to understand what you are trying to solve exactly here, can you provide further details please?

I assume it's because of:

result = self.pilotDB.selectPilots(
{"Status": PilotStatus.PILOT_TRANSIENT_STATES}, older=timeLimitToConsider, timeStamp="LastUpdateTime"
)
if not result["OK"]:
self.log.error("Failed to get the Pilot Agents")
return result
if not result["Value"]:
return S_OK()
refList = result["Value"]
result = self.pilotDB.getPilotInfo(refList)
if not result["OK"]:
self.log.error("Failed to get Info for Pilot Agents")
return result

If it's the case, I am wondering why we don't just simplify by having a getPilotInfoForSelection method (merging selectPilots and getPilotInfo into a single SQL request). Or even having selectPilots returning any pilot information? (that would make a breaking change in the API here)

@chaen

chaen commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

the problem is the query itself.

select * from Table where column in (1,2,3,4,...,126456464)

is not efficient.

On the other hand, this is very fast

create temporary table tmp_table(column INT PRIMARY KEY);
insert into tmp_table (column) values (1),(2),(3),(4),...,(126456464);
select * from table t join temporary table tt on t.column = tt.column;

Of course, it's for large number of values.

@aldbr

aldbr commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

the problem is the query itself.

select * from Table where column in (1,2,3,4,...,126456464)

is not efficient.

This I understand, but it looks like the query itself could be avoided by returning the pilot info instead of just the pilot reference from selectPilots(), unless I am missing some details

@fstagni

fstagni commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

the problem is the query itself.

select * from Table where column in (1,2,3,4,...,126456464)

is not efficient.

This I understand, but it looks like the query itself could be avoided by returning the pilot info instead of just the pilot reference from selectPilots(), unless I am missing some details

Apart from the call from the web, for which it might be true what you are saying, the general issue would still be there, right? @chaen when did you observe this? Which was the caller?

@chrisburr

Copy link
Copy Markdown
Member

IIRC the stalled job agent does a query like this?

@chaen

chaen commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

the problems were the site directors. I am happy with any change honestly, this was just an emergency fix

@aldbr aldbr changed the title feat: use temporary table instead of large IN query in PilotAgentsDB feat: get the PilotInfo in one query from PilotAgentsDB Oct 27, 2026
@aldbr
aldbr marked this pull request as ready for review October 27, 2026 16:10
@aldbr
aldbr requested review from atsareg and fstagni as code owners October 27, 2026 16:10
@aldbr

aldbr commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

I have changed the PR to just avoid the large IN by having 1 SQL request instead of 2.
I have tested it in lhcbdirac production, I haven't encountered any issue.
From a brief overview, it seems to improve the duration of the monitoring cycles but it doesn't seem to be significant.

chaen and others added 3 commits October 27, 2026 17:12
Add PilotAgentsDB.getPilotInfoForSelection, which returns the pilot
information for a selectPilots-style condition in a single query, and
attaches the jobs through a subquery on the same condition. SiteDirector
and PilotStatusAgent used selectPilots followed by getPilotInfo on the
returned references: above a few thousand references MySQL gives up the
index on the IN list and scans the whole PilotAgents table.

This replaces the temporary-table approach: getPilotInfo and
getJobsForPilot go back to their previous behaviour (S_OK/S_ERROR
contract, duplicate references tolerated).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aldbr
aldbr force-pushed the v9.1_feat_pilot_in_query branch from 5a10eac to 93d66ab Compare October 27, 2026 16:12
Comment thread src/DIRAC/WorkloadManagementSystem/DB/PilotAgentsDB.py Outdated
Comment thread src/DIRAC/WorkloadManagementSystem/DB/PilotAgentsDB.py Outdated
Comment on lines +346 to +347
if not result["OK"]:
return S_OK(resDict)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure?
If that's what you wanted, not even a warning here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I adjusted the function to done a single query instead of 2, the error is now returned to the caller

The jobs were selected in a second query that repeated the selection in a
subquery: it was not atomic with the first one, sent a reference list twice
and its failure was swallowed, leaving pilots without "Jobs" (PilotStatusAgent
would then kill pilots with running jobs).

- select the pilots in a derived table and LEFT JOIN JobToPilotMapping on it
- return the error instead of S_OK without "Jobs"
- getPilotInfoForSelection: add orderAttribute and limit, type hints
- getPilotInfo: paramNames defaults to None

Assisted-by: Claude:claude-opus-5-5
@fstagni
fstagni merged commit 026d141 into DIRACGrid:integration Oct 28, 2026
25 checks passed
@DIRACGridBot DIRACGridBot added the sweep:ignore Prevent sweeping from being ran for this PR label Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sweep:ignore Prevent sweeping from being ran for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants