-
Notifications
You must be signed in to change notification settings - Fork 230
store remoteAvailableShards to file #1663
store remoteAvailableShards to file #1663
Conversation
e4e3eb4
to
cdfcd6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great, one comment, but not a blocker
field.go
Outdated
|
||
// Write available shards to buffer. | ||
var buf bytes.Buffer | ||
if n, err := f.remoteAvailableShards.WriteTo(&buf); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why write to a buffer rather than directly to the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch. i removed it in ee503b6
// saveAvailableShards writes remoteAvailableShards data for the field. | ||
func (f *Field) saveAvailableShards() error { | ||
// Open or create file. | ||
file, err := os.OpenFile(filepath.Join(f.path, ".available.shards"), os.O_WRONLY|os.O_CREATE, 0666) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this truncate by default or should we pass that flag explicitly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it truncates by default. you have to include os.O_APPEND
to append.
Overview
This PR replaces the previous attempt (#1659) at making
remoteAvailableShards
immediately available after startup.Pull request checklist
Code review checklist
This is the checklist that the reviewer will follow while reviewing your pull request. You do not need to do anything with this checklist, but be aware of what the reviewer will be looking for.