Bug #5745

[Keep] keepstore should serialize file reads/writes, but not directory lookups, when -serialize=true

Added by Tom Clegg over 6 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Keep
Target version:
Start date:
05/07/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

background

The most obvious benefit of this change is that, even when keepstore is busy, the process of probing N volumes to find an existing block can proceed quickly, using the OS's filesystem cache, instead of waiting in every volume's serialize queue along the way. With the current implementation, a keepstore can easily accumulate a queue of PUT requests (each consuming a decent chunk of RAM) that are just waiting to find out whether the blocks already exist -- and of course they'll wait in the queue an N+1st time when it's time to actually write them to disk.

solution / implementation

Suggest refactoring the "serialize" implementation to use a sync.Mutex instead of passing channels to a goroutine over a "queue" channel.

In Read, before calling ioutil.ReadFile or doing any locking, do a stat() to see whether the file exists. If not, return an error without acquiring the volume lock.

In Read and Write, just before reading or writing data:

if v.mutex {
  v.mutex.Lock()
  defer v.mutex.Unlock()
}

Then we can rename Write to Put, rename Read to Get, and delete the old Get and Put wrappers, IOHandler, IORequest, IOResponse, and KeepGet/KeepPut constants.


Subtasks

Task #5924: review 5745-serialize-content-onlyResolvedTom Clegg

Associated revisions

Revision fde21d34
Added by Tom Clegg over 6 years ago

Merge branch '5745-serialize-content-only' closes #5745

History

#1 Updated by Tom Clegg over 6 years ago

  • Description updated (diff)
  • Category set to Keep

#2 Updated by Tom Clegg over 6 years ago

  • Target version changed from Arvados Future Sprints to 2015-05-20 sprint

#3 Updated by Tom Clegg over 6 years ago

  • Status changed from New to In Progress

#4 Updated by Peter Amstutz over 6 years ago

867263b 5745-serialize-content-only LGTM

#5 Updated by Peter Amstutz over 6 years ago

  • Assigned To set to Tom Clegg

#6 Updated by Tom Clegg over 6 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:fde21d34d011af2123668983c559632221390fd4.

Also available in: Atom PDF