Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kvserver: TestStoreMetrics is flakey #90631

Closed
ajwerner opened this issue Oct 25, 2022 · 2 comments · Fixed by #91166
Closed

kvserver: TestStoreMetrics is flakey #90631

ajwerner opened this issue Oct 25, 2022 · 2 comments · Fixed by #91166
Assignees
@coolcom200
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-test-failure Broken test (automatically or manually discovered). T-storage Storage Team

Comments

@ajwerner
Copy link
Contributor

ajwerner commented Oct 25, 2022 •
edited by exalate-issue-sync bot
Loading

Describe the problem

It's failed 3 in the last 20. https://teamcity.cockroachdb.com/test/-743047615087228185?currentProjectId=Cockroach_Ci_TestsGcpLinuxX8664BigVm&branch=%3Cdefault%3E&expandTestHistoryChartSection=true

Here's a failure: https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_UnitTests_BazelUnitTests/7125033?showRootCauses=false&expandBuildProblemsSection=true&expandBuildDeploymentsSection=true&expandBuildTestsSection=true

client_metrics_test.go:168: gauge rocksdb.table-readers-mem-estimate = 0 < min 50

Jira issue: CRDB-20866

Epic CRDB-20293
@ajwerner ajwerner added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-test-failure Broken test (automatically or manually discovered). labels Oct 25, 2022
@blathers-crl blathers-crl bot added the T-storage Storage Team label Oct 25, 2022
adityamaru added a commit to adityamaru/cockroach that referenced this issue Oct 25, 2022
@adityamaru
kv/kvserver: skip TestStoreMetrics
b026020 
Refs: cockroachdb#90631

Reason: flaky test

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None
@adityamaru adityamaru mentioned this issue Oct 25, 2022
Merged
@jbowens
Copy link
Collaborator

jbowens commented Oct 25, 2022 •
edited
Loading

rocksdb.table-readers-mem-estimate pulls from pebble.Metrics.TableCache.Size.

One avenue for investigation, could this is fallout from fixing #90149? I'm wondering if we might asynchronously set the cluster version, which triggers the migration to compact all the pre-Pebblev1 files. This could cause all the files that were just read to evicted from the table cache when they're deleted? not it

craig bot pushed a commit that referenced this issue Oct 25, 2022
@renatolabs @adityamaru
Merge #90478 #90632
847c146 
90478: server: write pebble log messages to storage channel r=knz a=renatolabs

Previously, when setting up the server, the pebble engine would be initialized with Pebble's default logger. The reason for this is that the pebble initialization code calls `EnsureDefaults` on the configuration options _before_ checking if the `options.Logger` is nil/unset. At that point, it will never be unset, as `EnsureDefaults` will set the logger to `pebble.DefaultLogger` if it was not previously set.

This change overwrites the pebble logger if its found to be the `DefaultLogger`. We never want to use pebble's `DefaultLogger` in CRDB as that would mean pebble would use the standard library `log` package, making every message emitted by Pebble to be treated as `INFO` level messages, regardless of severity (including `log.Fatal` calls).

Related to #83079.

Fixes #72683.
Fixes #90483.

Release note: None.

90632: kv/kvserver: skip TestStoreMetrics r=jbowens a=adityamaru

Refs: #90631

Reason: flaky test

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None

Co-authored-by: Renato Costa <[email protected]>
Co-authored-by: adityamaru <[email protected]>
@jbowens
Copy link
Collaborator

jbowens commented Oct 25, 2022

@bananabrick if you have time, can you triage this flake this week?

This was referenced Nov 7, 2022
Open
Merged
@craig craig bot closed this as completed in d9cc084 Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@coolcom200 coolcom200

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-test-failure Broken test (automatically or manually discovered). T-storage Storage Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kv/kvserver: fix TestStoreMetrics by stopping automatic compactions coolcom200/cockroach
4 participants
@jbowens @ajwerner @bananabrick @coolcom200