Fix global prefix not working with update marker #42
Conversation
|
I've been LS running with this patch since Thursday. The good news: it works. The better news: it reduces cost dramatically, so it works as intended. 
Now to the bad news: there's a memory (and maybe CPU as well) issue. I have a simple 2 node test setup, with 3 zones with very few updates, so just to test stability. Tje setup had been running fairly stable for about 10 days without the marker functionality. I then built a custom LS with this patched-simpleblob included, and enabled the marker feature on June 21st. As you can see from the graph below, a pattern started emerging at that time: there's a mem leak which starts to happen at that same time, leaking to very frequent OOMs.
Unfortunately I only have memory graphs from one of the nodes (it's a test setup), but do see the OOMs on the second node as well. The second node also exhibits increased CPU load. As you can see on the graphs, this behaviour only started when I enabled the marker-functionality, with this patch and it didn't happen before.
The 2 test servers are running:
|
|
Thank you for the detailed report! I created #44 to track this memory issue. I think I have found a culprit, that is related to GetObject operations, which matches the metrics you provided. |
|
@ahouene Seems to work: mem, cloud provider D: cpu, cloud provider V: |
backends/s3/s3.go
Outdated
| defer func() { | ||
| if !b.hasGlobalPrefix() { | ||
| return | ||
| } | ||
| for i, blob := range blobList { | ||
| s, _ := b.cutGlobalPrefix(blob.Name) | ||
| blobList[i].Name = s | ||
| } | ||
| }() |
There was a problem hiding this comment.
If I read this correctly, by putting all of this in a defer, you are changing the cached values and calling it over and over on existing cached values.
Don't do magic in defer, it will bite you.
ahouene
force-pushed
the
prefix-marker-fix
branch
from
72dcdd8 to
1b35697
Compare
When using both the update marker and the global prefix options, the marker would be written without the global prefix, and we'd try to read it with the global prefix.
This PR makes sure the global prefix is always used with the update marker.
Closes #40