feat(bidengine): support wildcard gpu multi-bids#415
Conversation
Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughCluster reservations now use ChangesBid-keyed reservations and multi-attempt bidding
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with 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.
Inline comments:
In `@bidengine/order_test.go`:
- Around line 249-271: The existing-bid mock in the QueryBids test setup is
inconsistent with the BidOpen filter, since it returns a BidLost result even
though the request only asks for open bids. Update the mock around
QueryBidsRequest/QueryBidsResponse and scaffold.marketMocks.On("Bids", ...) so
it returns a matching open bid only, or an empty response for non-open states,
and make the same adjustment in the duplicated test setup referenced by the same
QueryBidsRequest pattern.
In `@bidengine/order.go`:
- Around line 288-289: The early-exit paths in bidengine/order.go are leaving
the transferred runner in storedGroupCh unobserved after checkForExistingBid
moves groupch there. Update the shutdown/query-error/stale-bid exits in the
order flow around storedGroupCh and the main bid runner logic to explicitly
drain or consume storedGroupCh before returning, so the runner is always
observed even when the final drain path is skipped.
- Around line 480-487: The bid-close handling in the EventBidClosed path
currently removes an attempt, but the later ReserveBid completion can still
re-add that bid and resurrect a closed order. Update the attempt lifecycle
around findBidAttempt/removeAttempt and the reservation callback so closed bid
IDs are tracked as cancelled, and when a late clusterch result arrives for one
of those IDs, immediately unreserve and drop it instead of appending it back
with placed: reservationResult.existing. Apply the same cancelled-ID check in
the related reservation-result handling around the later attempt-processing
block.
- Around line 431-435: The lease-lost cleanup in bidengine/order.go is marking
broadcastingBid as placed too early, before the broadcast path has actually
succeeded. Update the handling around broadcastingBid in the affected
cleanup/broadcast flow (including the related bidch drain and the other
lease-lost paths) so attempts[idx].placed is only set after the broadcast
returns success, not while the bid is still in flight. Keep the logic consistent
across the related sections by using the existing findBidAttempt,
broadcastingBid, and attempts state transitions to avoid sending MsgCloseBid for
bids that were never created.
In `@cluster/inventory.go`:
- Around line 795-800: The IP confirmation loop is matching reservations by the
first `OrderID` hit, which can mark the wrong reservation when multiple bids
share an order; update the logic in the
`res.confirmedResult`/`state.reservations` iteration to identify the winning
reservation by `BidID` (or equivalent bid sequence metadata) instead of
`entry.OrderID()`, then set `ipsConfirmed` on that exact reservation and keep
the log aligned with the matched bid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 40e88500-caa4-447c-9a67-4b47bf28ab40
📒 Files selected for processing (7)
bidengine/order.gobidengine/order_test.gocluster/inventory.gocluster/reservation.gocluster/service.gomocks/cluster/Cluster_mock.gomocks/cluster/Service_mock.go
Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
|
addressed the review comments:
|
Signed-off-by: Joseph Chalabi <chalabi.joseph@gmail.com>
Summary
Adds provider multi-bid support for wildcard GPU model orders.
The bid engine now increments
BSeq, reserves per bid, submits per-bid resource offers, and frees losing reservations when one local bid wins.Tests
GOWORK=off go test ./...