Fix TSDB issues on Windows, add CI #6547
Conversation
brian-brazil
force-pushed
the
tsdb-win
branch
2 times, most recently
from
a752834
to
5d8c230
Compare
|
@brian-brazil does this need to be added to github repo config as well? |
|
This is me playing around, we don't have the right sort of CircleCI account for it though. |
|
you need to add it to jobs: as well |
brian-brazil
force-pushed
the
tsdb-win
branch
5 times, most recently
from
120df37
to
ab5e7ad
Compare
.circleci/config.yml
Outdated
| steps: | ||
| - checkout | ||
| - run: | ||
| - command: go test ./... |
.circleci/config.yml
Outdated
| working_directory: /go/src/github.com/prometheus/prometheus | ||
| steps: | ||
| - checkout | ||
| - run: go test ./... |
There was a problem hiding this comment.
| - run: go test ./... | |
| - run: | |
| command: go test ./... | |
| shell: bash.exe |
There was a problem hiding this comment.
mmh it works with powershell apparently.
brian-brazil
force-pushed
the
tsdb-win
branch
6 times, most recently
from
4c3a6b5
to
175c440
Compare
tsdb/index/index.go
Outdated
| @@ -720,6 +720,7 @@ func (w *Writer) writePostingsOffsetTable() error { | |||
| } | |||
|
|
|||
| // Cleanup temporary file. | |||
| f.Close() // Needed for Windows. | |||
There was a problem hiding this comment.
👍
f is mmap of fP0 so it's important to close this handler. I would say it's sane for non windows as well (: so maybe commnet not needed?
brian-brazil
force-pushed
the
tsdb-win
branch
9 times, most recently
from
2ecdd3a
to
c16d52b
Compare
brian-brazil
force-pushed
the
tsdb-win
branch
5 times, most recently
from
4922615
to
ffdd974
Compare
tsdb/index/index.go
Outdated
| if err := w.ensureStage(idxStageDone); err != nil { | ||
| return err | ||
| } | ||
| // Even if this failes, we need to close all the files. |
There was a problem hiding this comment.
| // Even if this failes, we need to close all the files. | |
| // Even if this fails, we need to close all the files. |
brian-brazil
force-pushed
the
tsdb-win
branch
2 times, most recently
from
8f75c50
to
11fe8cf
Compare
|
@simonpasquier @SuperQ This is touching CI, so one of ye should have a look at that bit. I did just enough to get it working. |
brian-brazil
force-pushed
the
tsdb-win
branch
6 times, most recently
from
6b82eae
to
9e474db
Compare
|
This is ready for review. |
tsdb/index/index.go
Outdated
| @@ -720,6 +724,8 @@ func (w *Writer) writePostingsOffsetTable() error { | |||
| } | |||
|
|
|||
| // Cleanup temporary file. | |||
| f.Close() // Needed for Windows. | |||
There was a problem hiding this comment.
Do we care about errors on close? (I know we used to not care) 🤔 should we log at least? It's a pretty unlikely error and not related to the bug we fix here, so not really necessary for this PR.
FYI we have this helper on Thanos: https://github.com/thanos-io/thanos/blob/865d5ec710b5651ead08a7dc73864a6f5dbeeaa9/pkg/runutil/runutil.go#L119
There was a problem hiding this comment.
We don't generally care, but I've returned it on the happy path here just in case.
Add back Windows CI, we lost it when tsdb was merged into the prometheus repo. There's many tests failing outside tsdb, so only test tsdb for now. Fixes #6513 Signed-off-by: Brian Brazil <brian.brazil@robustperception.io>
Add back Windows CI, we lost it when tsdb was merged into the prometheus repo. There's many tests failing outside tsdb, so only test tsdb for now. Fixes #6513 Signed-off-by: Brian Brazil <brian.brazil@robustperception.io>
No description provided.