Initial structure and block shipper #1
Conversation
Add a shipper to the sidecar command that scans the data directory and uploads new ULID named directories to a GCS bucket.
|
|
||
| format: | ||
| @echo ">> formatting code" | ||
| @go fmt ./... |
There was a problem hiding this comment.
Literally cannot get it to run (: But I guess leaving gofmt here is fine since missing or extra imports will cause things to not compile anyway, which CI will catch.
There was a problem hiding this comment.
yeah I was thinking about goimports just in terms of sorting imports (:
cmd/promlts/store.go
Outdated
|
|
||
| // registerStore registers a store command. | ||
| func registerStore(app *kingpin.Application, name string) runFunc { | ||
| cmd := app.Command(name, "sidecar for Prometheus server") |
There was a problem hiding this comment.
incorrect command description (copy-pasta from sidecar)
pkg/shipper/gcp.go
Outdated
| return false, err | ||
| } | ||
| // The first object found with the given filter indicates that the directory exists. | ||
| // XXX(fabxc): do a full check whether the files add up as well? |
There was a problem hiding this comment.
what do you mean add up? add up to what (:
There was a problem hiding this comment.
You mean if are files are there?
There was a problem hiding this comment.
Yea, basically just a bit more detailed check whether things are in sync.
There was a problem hiding this comment.
Well I think we can skip that now, it will only increase GCS operations but no one actually can write to this after upload now. Only when upload + dir delete was broken. Let's see how often that happens
| level.Error(r.logger).Log("msg", "upload failed; remove partial data", "dir", dir, "err", err) | ||
|
|
||
| // We don't want to leave partially uploaded directories behind. Cleanup everything related to it | ||
| // and use a uncanceled context. |
pkg/shipper/gcp.go
Outdated
|
|
||
| w := r.bucket.Object(target).NewWriter(ctx) | ||
| _, err = io.Copy(w, f) | ||
|
|
There was a problem hiding this comment.
kill newline? from the first glance err looks unchecked (:
pkg/shipper/shipper.go
Outdated
| return s.remote.Upload(ctx, dir) | ||
| } | ||
|
|
||
| // ReadDir returns the filenames in the given directory in sorted order. |
pkg/shipper/shipper.go
Outdated
| case <-tick.C: | ||
| names, err := readDir(s.dir) | ||
| if err != nil { | ||
| continue |
There was a problem hiding this comment.
Not even log here? We are losing error here
| func (s *Shipper) sync(ctx context.Context, dir string) error { | ||
| ok, err := s.remote.Exists(ctx, dir) | ||
| if err != nil { | ||
| return errors.Wrap(err, "check exists") |
There was a problem hiding this comment.
Shouldn't we just skip it? This will be spamming with log a lot on every case after some time, doesn't it?
There was a problem hiding this comment.
I'd only expect an error here if we failed to verify whether it is there or not. If it doesn't exist, err is still nil.
There was a problem hiding this comment.
ah, my bad, did not see ok, err := ):
| @@ -0,0 +1,67 @@ | |||
| // The MIT License (MIT) | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
He, fun story. So I actually rolled out those libs to a significant fraction of Prometheus code bases. We had a longer upstream discussion recently and concluded that the diffs it shows are rarely readable, which removes one expected benefit. Additionally, the rich equality checks were often behaving in unexpected ways. This caused us to work around it in non-obvious ways.
Overall, all we wanted was short versions for "no error occurred" and reflect.DeepEqual. Those few simple functions give us just that and are easy to use. Thus we decided to migrate Prometheus upstream to those.
There was a problem hiding this comment.
Additionally, the rich equality checks were often behaving in unexpected ways.
We did not notice these.. I guess only for some edge cases, right?
OK, so let's stick to these, I will try to limit usage to only these four ^^
docs: add initial design overview
| @@ -0,0 +1,67 @@ | |||
| // The MIT License (MIT) | |||
There was a problem hiding this comment.
Additionally, the rich equality checks were often behaving in unexpected ways.
We did not notice these.. I guess only for some edge cases, right?
OK, so let's stick to these, I will try to limit usage to only these four ^^
Disable 2 week compaction
add OWNERS file and basic Dockerfile
Delete mmapped chunks after creating block from head
…-func Reuse buffers for label comparison
@Bplotka This is mostly setup around the basic structure and some implementation to show what it would look like in practice. The block shipper is already working though.
A few design decisions/proposals:
vendor/cmd/andpkg/folder, no other Go packages at the top leveltestutilpackage for abbreviated checks