Skip to content

Conversation

codesome
Copy link
Contributor

@codesome codesome commented Oct 21, 2020

What this PR does:

Upgrades Prometheus to current master to get the following fixes in:

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@pracucci
Copy link
Contributor

@codesome FYI I pushed a fixed to a test.

pracucci
pracucci previously approved these changes Oct 22, 2020
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@pracucci
Copy link
Contributor

A test is segfaulting:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x11e3e06]

goroutine 14618 [running]:
github.com/prometheus/prometheus/tsdb.(*DB).Close(0x0, 0x0, 0x0)
	/go/src/github.com/cortexproject/cortex/vendor/github.com/prometheus/prometheus/tsdb/db.go:1328 +0x56
github.com/prometheus/prometheus/tsdb.open.func1(0xc0046f3a68, 0xc0046f3a60)
	/go/src/github.com/cortexproject/cortex/vendor/github.com/prometheus/prometheus/tsdb/db.go:606 +0x20f
github.com/prometheus/prometheus/tsdb.open(0xc001cf1040, 0x18, 0x22e8b80, 0xc00286aba0, 0x230e1a0, 0xc003035270, 0xc0030352c0, 0xc0070dd2c0, 0x1, 0xa, ...)
	/go/src/github.com/cortexproject/cortex/vendor/github.com/prometheus/prometheus/tsdb/db.go:651 +0x1208
github.com/prometheus/prometheus/tsdb.Open(0xc001cf1040, 0x18, 0x22e8b80, 0xc00286aba0, 0x230e1a0, 0xc003035270, 0xc0030352c0, 0xc00bfa9c70, 0x0, 0x0)
	/go/src/github.com/cortexproject/cortex/vendor/github.com/prometheus/prometheus/tsdb/db.go:530 +0xdc
github.com/cortexproject/cortex/pkg/ingester.(*Ingester).createTSDB(0xc009f36900, 0xc0031a82d3, 0x5, 0x11dbe0e, 0xc009a26180, 0xc006ac55f4)
	/go/src/github.com/cortexproject/cortex/pkg/ingester/ingester_v2.go:994 +0xb50
github.com/cortexproject/cortex/pkg/ingester.(*Ingester).openExistingTSDB.func1.1(0xc0031981f0, 0xc00c892bb8, 0xc009f36900, 0xc003198200, 0xc0042ba1e0, 0xc0031a82d3, 0x5)
	/go/src/github.com/cortexproject/cortex/pkg/ingester/ingester_v2.go:1147 +0x192
created by github.com/cortexproject/cortex/pkg/ingester.(*Ingester).openExistingTSDB.func1
	/go/src/github.com/cortexproject/cortex/pkg/ingester/ingester_v2.go:1140 +0x15c4
FAIL	github.com/cortexproject/cortex/pkg/ingester	38.469s
FAIL

@pracucci pracucci dismissed their stale review October 22, 2020 16:27

The panic is scary. Need to be investigated.

@codesome
Copy link
Contributor Author

I think prometheus/prometheus#8113 should fix the panic

@codesome
Copy link
Contributor Author

I have updated the PR to include prometheus/prometheus@3245b32, hope everything is fine this time 🤞

Ganesh Vernekar and others added 6 commits October 28, 2020 12:29
Signed-off-by: Ganesh Vernekar <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Ganesh Vernekar <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double checked the changes and LGTM! Only one comment: I think it would be worth to mention in the CHANGELOG the [BUGFIX] included in this upgrade, because they've a significative impact on Cortex users running the blocks storage.

Signed-off-by: Ganesh Vernekar <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
@csmarchbanks csmarchbanks merged commit 8f7a874 into cortexproject:master Oct 28, 2020
simonswine pushed a commit to grafana/e2e that referenced this pull request Jan 13, 2022
* Upgrade Prometheus to latest master

Signed-off-by: Ganesh Vernekar <[email protected]>

* Add another commit

Signed-off-by: Ganesh Vernekar <[email protected]>

* Fixed test

Signed-off-by: Marco Pracucci <[email protected]>

* Upgrade Prometheus again to fix the panic

Signed-off-by: Ganesh Vernekar <[email protected]>

* Fix tests

Signed-off-by: Ganesh Vernekar <[email protected]>

* Fixed test

Signed-off-by: Marco Pracucci <[email protected]>

* CHANGELOG entry

Signed-off-by: Ganesh Vernekar <[email protected]>

* Re-formatted CHANGELOG entry

Signed-off-by: Marco Pracucci <[email protected]>

Co-authored-by: Marco Pracucci <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants