fix repo id handling and PostgreSQL Query Ordering
Patch 1: PostgreSQL Query Ordering Fixed GetJobList() to return jobs ordered by job_repo_id Added JOIN with job_repo_map table to ensure proper ordering This ensures slice indices match the stored repository IDs Patch 2: Updated Comments Removed the TODO comment about hacky job IDing Added explanation that ordering is now consistent Patch 3: Added Validation Added runtime validation to catch ID mismatches Logs warnings/errors if slice index doesn't match expected job_repo_id Helps debug issues in different state implementations
This commit is contained in:
@@ -176,9 +176,23 @@ func (c *CommanderSingle) submitStatusResponse(w http.ResponseWriter, js common.
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Loop through all jobs under the same session id
|
// Loop through all jobs under the same session id
|
||||||
// TODO: as a high priority, fix this hacky job IDing by index
|
// The jobRepoId from the range should now match the stored job_repo_id
|
||||||
// this may break with other state implementations
|
// due to the ORDER BY job_repo_id in GetJobList()
|
||||||
for jobRepoId, job := range jobs {
|
for jobRepoId, job := range jobs {
|
||||||
|
// Validate that the slice index matches the expected repo ID
|
||||||
|
// This helps catch ordering issues between different state implementations
|
||||||
|
if expectedJobSpec, err := c.v.State.GetJobSpecByRepoId(js.SessionID, jobRepoId); err != nil {
|
||||||
|
slog.Warn("Job repo ID validation failed",
|
||||||
|
"sessionId", js.SessionID,
|
||||||
|
"jobRepoId", jobRepoId,
|
||||||
|
"error", err)
|
||||||
|
} else if expectedJobSpec.Owner != job.Spec.Owner || expectedJobSpec.Repo != job.Spec.Repo {
|
||||||
|
slog.Error("Job repo ID mismatch detected",
|
||||||
|
"sessionId", js.SessionID,
|
||||||
|
"jobRepoId", jobRepoId,
|
||||||
|
"expected", expectedJobSpec.NameWithOwner,
|
||||||
|
"actual", job.Spec.NameWithOwner)
|
||||||
|
}
|
||||||
// Get the job status
|
// Get the job status
|
||||||
status, err := c.v.State.GetStatus(job.Spec)
|
status, err := c.v.State.GetStatus(job.Spec)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -385,8 +385,11 @@ func (s *PGState) GetJobList(sessionId int) ([]queue.AnalyzeJob, error) {
|
|||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
rows, err := s.pool.Query(ctx, `
|
rows, err := s.pool.Query(ctx, `
|
||||||
SELECT payload FROM analyze_jobs
|
SELECT aj.payload FROM analyze_jobs aj
|
||||||
WHERE session_id = $1
|
JOIN job_repo_map jrm ON aj.session_id = jrm.session_id
|
||||||
|
AND aj.owner = jrm.owner AND aj.repo = jrm.repo
|
||||||
|
WHERE aj.session_id = $1
|
||||||
|
ORDER BY jrm.job_repo_id
|
||||||
`, sessionId)
|
`, sessionId)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
|
|||||||
Reference in New Issue
Block a user