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

Update from upstream repository incl. go1.20 compatibily patches #263

Merged
merged 22 commits into from
Feb 8, 2024

Conversation

periklis
Copy link

@periklis periklis commented Feb 8, 2024

chaudum and others added 22 commits February 5, 2024 21:48
… fetcher (grafana#11857)

**What this PR does / why we need it**:

1. The `BloomStore` interface gets a new function `FetchBlocks()`, which
accepts `[]BlockRef` and returns `[]BlockDirectory`.

   The fetcher implements a new function `FetchBlocks()` which returns a
   list of block directories.
   
   A block directory represents a local file path that contains the
   extracted files of a bloom block. It also holds a counter of active
   readers that access the directory. This is used for safely deleting the
   directory in case it needs to be removed from disk (eg. max disk/cache
   size reached).
   
   The fetcher resolves the block directory from three places:

   1. Cache: The in-memory cache that holds actively accessed directories
      and keeps track of used disk size.

   2. Filesystem: In case the cache was emptied (e.g. when restarting the
      process) but the block directory is present on disk, it can be
      re-assambled into a bloom directory that can be put to cache.

   3. Storage: If the directory is not present locally, the block archive
      is downloaded and extracted and the block directory is put to cache.

2. The `{Meta,Block}Client` interfaces are unified: Both clients define
the same set of operations:

       Get{Meta,Block}
       Get{Meta,Block}s
       Put{Meta,Block}
       Delete{Meta,Block}s

3. The `blockDownloader` is replaced by a simple queue powered by a
channel. The queue is implemented using generics, so it can be re-used
for different request/response types.

4. Code related to "block caching" moved into a separate file `cache.go`
and unused code is removed.

---------

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
Co-authored-by: Owen Diehl <ow.diehl@gmail.com>
…uantile vector pooling in Join function (grafana#11844)

This should reduce our memory usage for our quantile sketches by about
half, at least in our current situation we'll OOM after ~160-170 minutes
instead of ~90 😢

---------

Signed-off-by: Callum Styan <callumstyan@gmail.com>
Co-authored-by: Karsten Jeschkies <karsten.jeschkies@grafana.com>
**What this PR does / why we need it**:

This PR removes the `StoreAndClient` interface that was accepted by the
`BloomShipper`. Since the `BloomStore` had to not only implement the
`Store` interface, but also the `Client` interface, it caused
re-implementation of the same methods in different ways.

Now the shipper solely relies on the `Store` interface.

See individual commit messages for more context.

Tests have been rewritten from scratch and placed in their own
respective test files for store and client.

---------

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
Moves the archival code to an interface based approach, removing the
explicit fs dependency. We can now implement with in-memory solutions.
Also refactors to use a tar-file iterator and marks tests as
parallelizable.
Builds on top of grafana#11872,
standardizing on the `bloomshipper` types `Meta` and `MetaRef`.
**What this PR does / why we need it**:

* Removes `FetchBlocksWithQueue` and replace it with `FetchBlocks`
* Operate on single `BlockRef` when loading a block directory from
cache/fs/storage.
* Ensure order of responses from the FetchBlocks request

---------

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
**What this PR does / why we need it**:

For certain operations, we want direct access to the `BloomClient`.
Since every schema period has its own client, the `Client()` function
accepts a `model.Timestamp` for which the client should be returned.

The function may return an error, if no client for the given time could
be found.

---------

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
…leBlockQuerier` (grafana#11884)

**What this PR does / why we need it**:

In order to encapsulate the reference counting on the `BlockDirectory`,
the `BloomStore` now returns a slice of `*CloseableBlockQuerier` instead
of the directory itself.

The caller is responsible for closing the returned querier in order to
release the reader resource and decrement the ref counter.

The PR also renames `ClosableBlockQuerier` to `CloseableBlockQuerier`.

---------

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
**What this PR does / why we need it**:
Updating the docs in advance of OTEL release to remove "experimental"
notes.

Also restructured release notes so that most recent content is first.
…#11886)

Builds a lazy iterator over a tsdb file for bloom construction. This is
just the library utility, not the integration.
**What this PR does / why we need it**:
changed first person to comply with style guide

**Which issue(s) this PR fixes**:
Fixes #<issue number>
No issue

**Special notes for your reviewer**:

**Checklist**
- [ ] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [ ] Documentation added
- [ ] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [ ] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](grafana@d10549e)
- [ ] If the change is deprecating or removing a configuration option,
update the `deprecated-config.yaml` and `deleted-config.yaml` files
respectively in the `tools/deprecated-config-checker` directory.
[Example
PR](grafana@0d4416a)
**What this PR does / why we need it**:
We inject the downstream accumulator into the downstreamer instead of
constructing it on the first arrival of the first result.

Since we know the query type before executing the first query we can
pass the correct accumulator. This will allow us to define special
`topk` or `sum by` accumulators in the future.

**Checklist**
- [ ] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [ ] Documentation added
- [ ] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [ ] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](grafana@d10549e)
- [ ] If the change is deprecating or removing a configuration option,
update the `deprecated-config.yaml` and `deleted-config.yaml` files
respectively in the `tools/deprecated-config-checker` directory.
[Example
PR](grafana@0d4416a)

---------

Co-authored-by: Christian Haudum <christian.haudum@gmail.com>
This PR replaces the iteration of the bloom gateway worker with the code
encapsulated in the previously established processor.

Since the processor only relies on the BloomStore, rather than on the
BloomShipper, the PR also removes unused code from the shipper.


Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
Until now, the cache configuration in the bloom gateway did not initialise the cache.
This PR wires the config with the actual implementation and passes them to the bloom store.

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
A few changes wiring up bloom building logic:
* A `Router` as the controlling mechanism for iterating tenants+periods,
queueing work for parallel computation, determining which fingerprint
ranges and tenants are owned by a replica, and waiting for it's
completion
* Updates the `SimplerBloomController` to accept ownership ranges,
periods, and tenants as arguments for use by the `Router`
* Builds a `BloomTSDBStore` struct around the
`indexshipper/storage.Client` interface in order to iterate relevant
TSDBs/etc
…rafana#11902)

Recent changes causing compilation failures -- this fixes type
signatures after refactoring.

Also removes test coverage comparisons because they prevent us from
fixing `main`
…11901)

Rather than call all our values together, this PR precomputes a few in
`metrics.go` so any issues will result in smaller stack traces.
@periklis periklis self-assigned this Feb 8, 2024
Copy link

openshift-ci bot commented Feb 8, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: periklis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 8, 2024
Copy link

openshift-ci bot commented Feb 8, 2024

@periklis: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@periklis periklis merged commit cbcf476 into openshift:main Feb 8, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants