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.

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

[dbnode] Evict info files cache before index bootstrap in peers bootstrapper #2802

Merged
merged 3 commits into from
Oct 28, 2020

Conversation

notbdu
Copy link
Contributor

@notbdu notbdu commented Oct 27, 2020

What this PR does / why we need it:

The info files cache becomes stale after the peers bootstrapper fetches/persists TSDB blocks streamed from peers.

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:

NONE

Does this PR require updating code package or user-facing documentation?:

NONE

Copy link
Collaborator

@nbroyles nbroyles left a comment

Choose a reason for hiding this comment

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

LGTM pending mock regen

@notbdu notbdu merged commit 9bc596d into master Oct 28, 2020
@notbdu notbdu deleted the bdu/peersbs-fix branch October 28, 2020 05:05
@@ -185,6 +185,9 @@ func (s *peersSource) Read(
continue
}

// NB(bodu): We need to evict the info file cache before reading index data since we've
// maybe fetched blocks from peers so the cached info file state is now stale.
cache.Evict()
Copy link
Collaborator

@robskillington robskillington Oct 28, 2020

Choose a reason for hiding this comment

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

Think we just want to evict before iterating all namespaces so we can just evict once before processing all namespaces rather than evicting each time before processing an individual namespace?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yeah, missed that this was in a loop over the namespaces. We definitely only want to to do this once.

soundvibe added a commit that referenced this pull request Oct 29, 2020
* master:
  Support dynamic namespace resolution for embedded coordinators (#2815)
  [dbnode] Add index regexp DFA compilation cache to avoid allocating DFAs for same expressions (#2814)
  [dbnode] Set default cache on retrieve to false, prepare testing with cache none (#2813)
  [tools] Output annotations as base64 (#2743)
  Add Reset Transformation (#2794)
  [large-tiles] Fix for a reverse index when querying downsampled namespace (#2808)
  [dbnode] Evict info files cache before index bootstrap in peers bootstrapper (#2802)
  [dbnode] Bump default filesystem persist rate limit (#2806)
  [coordinator] Add metrics and configurability of downsample and ingest writer worker pool (#2797)
  [dbnode] Fix concurrency granularity of seekerManager.UpdateOpenLease (#2790)
  [dbnode] Use correct import path for atomic library (#2801)
  Regenerate carbon automapping rules on namespaces changes (#2793)
  Enforce matching retention and index block size (#2783)
  [query] Add ClusterNamespacesWatcher and use to generate downsample automapper rules (#2782)
  [query][dbnode] Wire up etcd-backed Clusters implementation in coordinator (#2758)
  [query] Fix quantile() argument not being passed through (#2780)

# Conflicts:
#	src/query/parser/promql/matchers.go
soundvibe added a commit that referenced this pull request Oct 29, 2020
* master:
  [query] Improve precision for variance and stddev of equal values (#2799)
  Support dynamic namespace resolution for embedded coordinators (#2815)
  [dbnode] Add index regexp DFA compilation cache to avoid allocating DFAs for same expressions (#2814)
  [dbnode] Set default cache on retrieve to false, prepare testing with cache none (#2813)
  [tools] Output annotations as base64 (#2743)
  Add Reset Transformation (#2794)
  [large-tiles] Fix for a reverse index when querying downsampled namespace (#2808)
  [dbnode] Evict info files cache before index bootstrap in peers bootstrapper (#2802)
  [dbnode] Bump default filesystem persist rate limit (#2806)
  [coordinator] Add metrics and configurability of downsample and ingest writer worker pool (#2797)
  [dbnode] Fix concurrency granularity of seekerManager.UpdateOpenLease (#2790)
  [dbnode] Use correct import path for atomic library (#2801)
  Regenerate carbon automapping rules on namespaces changes (#2793)
  Enforce matching retention and index block size (#2783)
  [query] Add ClusterNamespacesWatcher and use to generate downsample automapper rules (#2782)
  [query][dbnode] Wire up etcd-backed Clusters implementation in coordinator (#2758)
  [query] Fix quantile() argument not being passed through (#2780)
  [query] Add "median" aggregation to Graphite aggregate() function (#2774)
  [r2] Ensure KeepOriginal is propagated when adding/reviving rules (#2796)
  [dbnode] Update default db read limits  (#2784)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants