combine write and update to one request #6
Conversation
Signed-off-by: Babis Kiosidis <[email protected]>
| w.Close() | ||
|
|
||
| attr, err := obj.Update(c.ctx, storage.ObjectAttrsToUpdate{Metadata: mdPrepped}) |
There was a problem hiding this comment.
I wonder if the errors we are seeing on some RPCs are because we are not checking err = w.Close()
| @@ -114,17 +114,25 @@ func (c *Container) Put(name string, r io.Reader, size int64, metadata map[strin | |||
| } | |||
|
|
|||
| w := obj.NewWriter(c.ctx) | |||
| w.ObjectAttrs.Metadata = merge(w.ObjectAttrs.Metadata, mdPrepped) | |||
There was a problem hiding this comment.
The obj.Update does checks arround attrs and for empty string for certain fields eg :
https://github.com/googleapis/google-cloud-go/blob/5a8c607e3a9a3265887e27cb13f8943f3e3fa23d/storage/storage.go#L958
And does workaround by setting rawObj.NullFields = nullFields
Will that cause an issue if we now dont use the Update call
There was a problem hiding this comment.
We only set the metadata in this change.
The rest of the ObjectAttrs we leave as is (which is the same as the previous behaviour)
|
|
||
| func merge(metadata ...map[string]string) map[string]string { | ||
| res := map[string]string{} | ||
| for _, mt := range metadata { |
There was a problem hiding this comment.
If the Metadata is key/value pairs according to google documentation. Why do we need the nested loop? Shouldn't we just:
for k, v := range metadata {
res[k] = v
}
?
There was a problem hiding this comment.
This merges a list of maps into one... so the first loop iterates over the list of maps, the second loop iterates over the keys/values in each of them..
Signed-off-by: Babis Kiosidis [email protected]
Context
We are noticing about 10% errs when talking to gcs, with 404 and 429 (high rate of updates) error codes.
From Google quotas docs
There is a write limit to the same object name. This limit is once per second, so writes to the same object name won't scale. For more information, see [Object immutability](https://cloud.google.com/storage/docs/key-terms#immutability).Currently it seems that stow will create and update a gcs file when it can be done on a single operation.
The google-go library suggests to set attributes on the Writer and the Attrs() call should contain the necessary metadata according to the comment
What would be the best way to test this change? Any tips?
cc @EngHabu
will contribute this back to graymeta/stow if merged