Arvados

Simplified version ready for review at 199d58a4df0dba1ee71c6816bc3a9d9d439cfd7e on branch 16585-keep-exercise-improvements

Added -useIndex flag to rule out caching issues. Ready for review at 34cb31d5191ce17c37ddd9d344f4d77125af64a8 on branch 16585-keep-exercise-improvements

recommend changing -useIndex flag to -use-index to match other flags

fmt.Print("\r") after ^C should probably print to stderr instead of stdout

typo UseIndx in comment

seems weird to see stderr as the name of a logger -- how about log or lgr?

getIndexLocators() and doIndexReads() - instead of putting the entire func in an if ctx.Err()==nil, just put if ctx.Err() != nil { return } at the top and un-indent the rest

Similarly you could do for i := 0; i < *Repeat && ctx.Err() == nil; i++ { in main()

At the end of main, maybe it's better not to skip the summary when ctx.Err()!=nil, i.e., exiting on a signal?

Having two copies of doIndexReads/doReads doesn't seem ideal. Maybe something like this could handle both cases, and the caller would just provide a nil indexLocatorChan if !*UseIndex?

for ctx.Err() == nil {
  var locator string
  if indexLocatorChan != nil {
    select {
    case locator = <- indexLocatorChan:
    case <-ctx.Done():
      return
    }
  } else {
    locator = nextLocator.Load().(string)
  }
  // ...

Is it worth deduplicating the keepstore index results, to improve cache-avoidance a bit?

Tom Clegg wrote:

...

I've implemented all these changes, ready for another look at 866decb73a9cf07b28e26c5028a1d42a5ef243a7 on branch 16585-keep-exercise-improvements.

LGTM, thanks