Update from upstream repository #152
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…SDB index enabled (#9597) **What this PR does / why we need it**: set TSDB shipper mode to ReadOnly and disabled indexGatewayClient during local query run. Also, I increase index downloading timeout to 1m because the users run the query from local machines that not always have more than 100Mbps **Which issue(s) this PR fixes**: Fixes #9555 **Special notes for your reviewer**: Unfortunately, we do not have similar tests and test architecture to cover it with unit tests properly. **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [ ] Documentation added - [ ] Tests updated - [x] `CHANGELOG.md` updated - [x] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md` - [x] 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](d10549e)
**What this PR does / why we need it**: Before this PR, the index stats cache would use the same config as the query results cache. This was a limitation since: 1. We would not be able to point to a different cache for storing the index stats if needed. 2. We would not be able to add specific settings for this cache, without adding it to the results cache. In this PR, we refactor the index stats cache config to be independently configurable. Note that if it's not configured, it will try to use the results cache settings. **Which issue(s) this PR fixes**: This is needed for: - #9537 - #9536 **Special notes for your reviewer**: - This PR also refactors all the tripperwares in rountrip.go to reuse the same stats tripperware instead of each one creating their own. - Configuring a new cache in rountrip.go is a requirement for #9536 so the stats summary can distinguish before the stats cache and the results cache. **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [x] Documentation added - [x] Tests updated - [x] `CHANGELOG.md` updated - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_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](d10549e)
Co-authored-by: Periklis Tsirakidis <periklis@redhat.com>
|
[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 |
openshift-ci
bot
added
the
approved
**What this PR does / why we need it**:
When a query finishes, we return (and log) the following stats:
```go
Cache.Chunk.Requests 0
Cache.Chunk.EntriesRequested 0
Cache.Chunk.EntriesFound 0
Cache.Chunk.EntriesStored 0
Cache.Chunk.BytesSent 0 B
Cache.Chunk.BytesReceived 0 B
Cache.Chunk.DownloadTime 0s
Cache.Index.Requests 0
Cache.Index.EntriesRequested 0
Cache.Index.EntriesFound 0
Cache.Index.EntriesStored 0
Cache.Index.BytesSent 0 B
Cache.Index.BytesReceived 0 B
Cache.Index.DownloadTime 0s
Cache.Result.Requests 13
Cache.Result.EntriesRequested 13
Cache.Result.EntriesFound 13
Cache.Result.EntriesStored 0
Cache.Result.BytesSent 0 B
Cache.Result.BytesReceived 2.5 kB
Cache.Result.DownloadTime 4.600266ms
```
In addition to that, we log the following in metrics.go:
```
level=info ts=2023-05-29T09:17:10.93029945Z caller=metrics.go:152 component=frontend org_id=145265 traceID=52d59b78fe6b9221 sampled=true latency=fast query="{cluster=\"dev-us-central-0\", namespace=~\"loki.*\", container=~\"distributor|ingester
|promtail|index-gateway|compactor\"} |= \"thislinewillnotexist\"" query_hash=1194136170 query_type=filter range_type=range length=3h0m0s start_delta=165h37m24.930289434s end_delta=162h37m24.930289612s step=43s duration=2.473055ms status=200 lim
it=30 returned_lines=0 throughput=0B total_bytes=0B lines_per_second=0 total_lines=0 total_entries=0 store_chunks_download_time=0s queue_time=0s splits=13 shards=0 cache_chunk_req=0 cache_chunk_hit=0 cache_chunk_bytes_stored=0 cache_chunk_bytes
_fetched=0 cache_chunk_download_time=0s cache_index_req=0 cache_index_hit=0 cache_index_download_time=0s cache_result_req=13 cache_result_hit=13 cache_result_download_time=4.600266ms
```
With the goal of being able to better monitor how the stats cache is
performing; this PR adds stats for the index stats cache, similarly to
how it's done for the results cache.
Here's an example of the new stats being returned and printed:
```go
...
Cache.StatsResult.Requests 180
Cache.StatsResult.EntriesRequested 129
Cache.StatsResult.EntriesFound 129
Cache.StatsResult.EntriesStored 51
Cache.StatsResult.BytesSent 0 B
Cache.StatsResult.BytesReceived 75 kB
...
```
And the new stats from metrics.go
```
... caller=metrics.go:155 ... cache_stats_results_req=129 cache_stats_results_hit=129 cache_stats_results_download_ti
me=156.864429ms ...
```
**Special notes for your reviewer**:
- Blocked by #9535
- Note the new`stats.GetOrCreateContext` func. It's used inside the
`query.Exec` method so we don't overwrite the stats added in the stats
middleware.
**Checklist**
- [x] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [ ] Documentation added
- [x] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/upgrading/_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](d10549e)
|
@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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref: