Project

General

Profile

Actions

Feature #9395

closed

[keep-balance] fetch more collections per page

Added by Tom Clegg almost 8 years ago. Updated almost 8 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Keep
Target version:
Story points:
0.5

Description

Currently keep-balance has a hard-coded page size of 1000, which corresponds the default value of the API config max_items_per_response.

This means keep-balance will fetch lots of small pages even when a much larger max_items_per_response is configured.

It would be better for keep-balance to have at least one of:
  • a much bigger default (perhaps maxint) for the normal case where max_items_per_response is the desired page size
  • a configuration setting, in case an admin wants to use a smaller page size for keep-balance even though max_items_per_response is large

Subtasks 1 (0 open1 closed)

Task #9396: Review 9395-keep-balance-page-sizeResolvedRadhika Chippada06/13/2016Actions
Actions #1

Updated by Tom Clegg almost 8 years ago

9395-keep-balance-page-size @ 177b3b6 implements:
  • change default from 1000 to biggest page size offered by API
  • add page size config

Also, a related feature: The default 1000-collection buffer could consume a lot of memory if it receives many large collections, even if they're received in multiple pages. Added a config to control the buffer size. Default is now zero, which should reduce memory use but run a bit slower than the previous hard-coded 1000.

Actions #2

Updated by Tom Clegg almost 8 years ago

  • Status changed from New to In Progress
  • Assigned To set to Tom Clegg
Actions #3

Updated by Brett Smith almost 8 years ago

  • Target version set to 2016-06-22 sprint
Actions #4

Updated by Radhika Chippada almost 8 years ago

Review comments at 177b3b66

  • GetCurrentState -> Is the passed in “bufs” (CollectionBuffers) being used?
  • usage.go -> can you please add explanation about CollectionBatchSize and CollectionBuffers here?
  • One test failing with run-tests
    FAIL: integration_test.go:72: integrationSuite.TestBalanceAPIFixtures
    
    integration_test.go:82:
        c.Check(err, check.IsNil)
    ... value *errors.errorString = &errors.errorString{s:"request failed (https://0.0.0.0:43141/arvados/v1/collections?limit=2.147483647e%2B09&order=modified_at%2C+uuid&select=%5B%22uuid%22%2C%22manifest_text%22%2C%22modified_at%22%2C%22portable_data_hash%22%2C%22replication_desired%22%5D): 422 Unprocessable Entity"} ("request failed (https://0.0.0.0:43141/arvados/v1/collections?limit=2.147483647e%2B09&order=modified_at%2C+uuid&select=%5B%22uuid%22%2C%22manifest_text%22%2C%22modified_at%22%2C%22portable_data_hash%22%2C%22replication_desired%22%5D): 422 Unprocessable Entity")
    
    integration_test.go:84:
        c.Check(logBuf.String(), check.Matches, `(?ms).*ChangeSet{Pulls:1.*`)
    ...
    
Actions #5

Updated by Tom Clegg almost 8 years ago

  • GetCurrentState -> Is the passed in “bufs” (CollectionBuffers) being used?

Oops, git operator error. Now actually uses bufs instead of 1000.

  • usage.go -> can you please add explanation about CollectionBatchSize and CollectionBuffers here?

Added.

Working on SDK bug causing test failure...

Actions #6

Updated by Tom Clegg almost 8 years ago

Tom Clegg wrote:

Working on SDK bug causing test failure...

Fixed in 7b6d593

Actions #7

Updated by Radhika Chippada almost 8 years ago

LGTM

Actions #8

Updated by Tom Clegg almost 8 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:76be616a8a65a6c574026583c462640dcc9e706f.

Actions

Also available in: Atom PDF