[bugfix] logql: vectorExpr run fail when query_range: parallelise_shardable_queries: true #7045
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
…rdable_queries: true
kavirajk
reviewed
Sep 5, 2022
👌 |
done,thanks. |
kavirajk
reviewed
Sep 5, 2022
| @@ -782,6 +782,67 @@ func TestMapping(t *testing.T) { | |||
| }, | |||
| }, | |||
| }, | |||
| { | |||
| in: `vector(0) or sum (rate({foo="bar"}[5m]))`, | |||
There was a problem hiding this comment.
This test seems to be failing.
https://drone.grafana.net/grafana/loki/15228/2/5
There was a problem hiding this comment.
The original testcase is indeed wrong, and the 'Group' field is not considered.
I just submitted a commit to fix this testcase
|
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
kavirajk
approved these changes
Sep 22, 2022
|
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
5 tasks
ssncferreira
added a commit
that referenced
this pull request
Sep 29, 2022
**What this PR does / why we need it**: The new expression type VectorExpr introduced recently in [PR](#7007), is not taken into account in the query mapping in the AST mapper rangemapper. The AST mapper shardmapper was updated in [PR](#7045). **Special notes for your reviewer**: **Checklist** - [ ] Reviewed the `CONTRIBUTING.md` guide - [ ] 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`
lxwzy
pushed a commit
to lxwzy/loki
that referenced
this pull request
Nov 7, 2022
…rdable_queries: true (grafana#7045) …hardable_queries: true <!-- Thanks for sending a pull request! Before submitting: 1. Read our CONTRIBUTING.md guide 2. Name your PR as `<Feature Area>: Describe your change`. a. Do not end the title with punctuation. It will be added in the changelog. b. Start with an imperative verb. Example: Fix the latency between System A and System B. c. Use sentence case, not title case. d. Use a complete phrase or sentence. The PR title will appear in a changelog, so help other people understand what your change will be. 3. Rebase your PR if it gets out of sync with main --> **What this PR does / why we need it**: A grafana error occurred when I deployed the vector(0) function to our online loki cluster. It was finally found because online loki has a configuration `unexpected expr type (*syntax.VectorExpr) for ASTMapper type (logql.ShardMapper) ` ```yaml query_range: parallelise_shardable_queries: true ``` **Which issue(s) this PR fixes**: Fixes # **Special notes for your reviewer**: error snapssnapshot  with this pr，success snapshot  <!-- Note about CHANGELOG entries, if a change adds: * an important feature * fixes an issue present in a previous release, * causes a change in operation that would be useful for an operator of Loki to know then please add a CHANGELOG entry. For documentation changes, build changes, simple fixes etc please skip this step. We are attempting to curate a changelog of the most relevant and important changes to be easier to ingest by end users of Loki. Note about the upgrade guide, if this changes: * default configuration values * metric names or label names * changes existing log lines such as the metrics.go query output line * configuration parameters * anything to do with any API * any other change that would require special attention or extra steps to upgrade Please document clearly what changed AND what needs to be done in the upgrade guide. --> **Checklist** - [ ] Documentation added - [ ] Tests updated - [ ] Is this an important fix or new feature? Add an entry in the `CHANGELOG.md`. - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md`
lxwzy
pushed a commit
to lxwzy/loki
that referenced
this pull request
Nov 7, 2022
**What this PR does / why we need it**: The new expression type VectorExpr introduced recently in [PR](grafana#7007), is not taken into account in the query mapping in the AST mapper rangemapper. The AST mapper shardmapper was updated in [PR](grafana#7045). **Special notes for your reviewer**: **Checklist** - [ ] Reviewed the `CONTRIBUTING.md` guide - [ ] 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`
changhyuni
pushed a commit
to changhyuni/loki
that referenced
this pull request
Nov 8, 2022
**What this PR does / why we need it**: The new expression type VectorExpr introduced recently in [PR](grafana#7007), is not taken into account in the query mapping in the AST mapper rangemapper. The AST mapper shardmapper was updated in [PR](grafana#7045). **Special notes for your reviewer**: **Checklist** - [ ] Reviewed the `CONTRIBUTING.md` guide - [ ] 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`
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.
…hardable_queries: true
What this PR does / why we need it:
A grafana error occurred when I deployed the vector(0) function to our online loki cluster.
It was finally found because online loki has a configuration
unexpected expr type (*syntax.VectorExpr) for ASTMapper type (logql.ShardMapper)Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
error snapssnapshot
with this pr，success snapshot
Checklist
CHANGELOG.md.docs/sources/upgrading/_index.md