Project

General

Profile

Actions
Copy link

Bug #22400

closed

broken paging logic causes arvados-dispatch-cloud to hang at startup while looking up spot prices

Added by Tom Clegg 5 days ago. Updated 3 days ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Tom Clegg
Category:
Crunch
Target version:
Development 2025-01-08
Story points:
-
Release:
Arvados 3.0.1
Release relationship:
Auto

Description

We have

func (instanceSet *ec2InstanceSet) updateSpotPrices(instances []cloud.Instance) {
...
                if page.NextToken == nil {
                        break
                }

but the NextToken documentation is

        // The token to include in another request to get the next page of items. This                                                                                            
        // value is an empty string ( "" ) or null when there are no more items to return.                                                                                        
        NextToken *string

The web docs also mention this.

Subtasks 1 (0 open — 1 closed)

Task #22403: Review 22400-spot-price-pagingResolvedTom Clegg12/18/2024Actions

Related issues 2 (2 open — 0 closed)

Related to Arvados - Bug #22402: arvados-dispatch-cloud should log warnings when cloud API calls are slowNewTom CleggActions
Related to Arvados - Bug #22401: crunch-run --list (with no additional args) should not read from stdinNewTom CleggActions
Actions
Copy link
 #1

Updated by Tom Clegg 5 days ago

Other similar structs in the AWS SDK (DescribeInstancesOutput, DescribeInstanceStatusOutput) do not allow the "" case:

        // The token to include in another request to get the next page of items. This                                                                                            
        // value is null when there are no more items to return.                                                                                                                  
        NextToken *string

But "" isn't plausible as a "next page" token, so we should probably update our uses of those APIs too while we're at it.

Actions
Copy link
 #2

Updated by Tom Clegg 5 days ago

  • Description updated (diff)
Actions
Copy link
 #4

Updated by Tom Clegg 5 days ago

  • Related to Bug #22402: arvados-dispatch-cloud should log warnings when cloud API calls are slow added
Actions
Copy link
 #5

Updated by Tom Clegg 5 days ago

  • Related to Bug #22401: crunch-run --list (with no additional args) should not read from stdin added
Actions
Copy link
 #6

Updated by Lucas Di Pentima 4 days ago

This LGTM, thanks.

Actions
Copy link
 #7

Updated by Tom Clegg 4 days ago

  • Release set to 75
Actions
Copy link
 #8

Updated by Tom Clegg 4 days ago

  • Status changed from In Progress to Resolved
Actions
Copy link

Also available in: Atom PDF