enqueue-cheat-check tool + faster Hermes backlog autoscaling - #65
Open
owens1127 wants to merge 2 commits into
Open
enqueue-cheat-check tool + faster Hermes backlog autoscaling#65owens1127 wants to merge 2 commits into
owens1127 wants to merge 2 commits into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
| } | ||
|
|
||
| if len(p.activities) > 0 { | ||
| conds = append(conds, fmt.Sprintf("av.activity_id = ANY($%d)", n)) |
There was a problem hiding this comment.
Bug: When -instance-ids is used with -activity or -version, the code applies both filters, contradicting the flag's documentation which states activity/version filters should be skipped.
Severity: MEDIUM
Suggested Fix
Modify the logic in buildQuery to only perform the join and filter for activities/versions when no instance IDs are provided. Change the assignment of joinActivity to len(p.instanceIDs) == 0 && (len(p.activities) > 0 || len(p.versions) > 0). This will align the implementation with the documented behavior.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: tools/enqueue-cheat-check/main.go#L195
Potential issue: There is a logical contradiction between the documented behavior of the
`-instance-ids` flag and its implementation in the `buildQuery` function. The flag's
help text states it "skips activity/version filters". However, if a user provides both
`-instance-ids` and `-activity` or `-version` flags, the code generates a SQL query that
filters on both instance IDs and activity/version. This results in silently returning
fewer instances than the user expects based on the documentation. The issue stems from
the `joinActivity` variable being set to true even when instance IDs are present,
leading to an incorrect query.
Did we get this right? 👍 / 👎 to inform future reviews.
Add LargeBacklogThreshold fast-path and 30s depth checks for instance_cheat_check, player_crawl, and character_fill. Co-authored-by: Cursor <cursoragent@cursor.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tools/enqueue-cheat-checkto publish instance IDs toinstance_cheat_checkfor Hermes rechecks (activity, version, date range, instance IDs, skip-current, limit, dry-run).LargeBacklogThresholdfast-path intopic_manager— skips consecutive-check wait when depth is high.ScaleCheckInterval(was 5 min default) oninstance_cheat_check,player_crawl,character_fill.Autoscaling changes
instance_cheat_checkplayer_crawlcharacter_fillUsage
Deploy
Prod note
enqueue-cheat-checkalready used on prod to enqueue 819,743 activity-102 instances (tool scp'd pre-merge). Hermes autoscaling fix requires restart after merge.Test plan
go build ./apps/hermes/...go build ./tools/enqueue-cheat-check/-activity=102instance_cheat_checkworkers scale up within ~30s on deep queue