Frontend Refactor #3400
Frontend Refactor #3400
Conversation
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
joe-elliott
requested review from
annanay25,
mdisibio,
mapno,
kvrhdn,
zalegrala,
electron0zero,
ie-pham and
stoewer
as code owners
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
|
@joe-elliott Was there doc added? I didn't see any in the file list to review. |
There was not. This change is invisible to the user. Unchecking the docs item! |
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
|
|
||
| go func() { | ||
| if resps != nil { | ||
| asyncResp.Send(resps) |
There was a problem hiding this comment.
Could you help me understand more about what resps is? My understanding is that asyncResp.Send(resp) above is the core data flow, sending things up the chain as they come in. This looks like a final send that's already known by the caller?
There was a problem hiding this comment.
this was added to allow the sharder to craft responses unrelated to a request and send them back up the chain. in this case it's used to communicate search job size data up to the combiner.
processed here:
https://github.com/grafana/tempo/pull/3400/files#diff-96cf01ab28f4274fe65e4501fbf82cd479266202d7bc766d3a7c720475568cdaR38-R41
this highlights the largest complication with this PR. code that used to do the recombining was highly context aware:
https://github.com/grafana/tempo/blob/main/modules/frontend/searchsharding.go#L245-L318
b/c it was just executed after all jobs had completed in the same function. Now we are pushing all jobs up to the collector level to stream results. As a result some of that context is lost and it creates "message passing" code like this.
There was a problem hiding this comment.
Got it, thanks. Caching is another instance, where the cacheKey is passed through the context.
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
|
|
||
| go func() { | ||
| if resps != nil { | ||
| asyncResp.Send(resps) |
There was a problem hiding this comment.
Got it, thanks. Caching is another instance, where the cacheKey is passed through the context.
| } | ||
|
|
||
| // fetch fetches the response body from the cache. the caller assumes the responsibility of closing the response body. | ||
| func (c *frontendCache) fetch(key string, pb proto.Message) bool { |
There was a problem hiding this comment.
This method looks unused except for the test, and wasn't sure about json+proto.Message. Since we know it's a proto.Message we could store the binary representation in cache instead.
There was a problem hiding this comment.
yup. i do think there's an opportunity to make the sync part of the pipeline type aware with generics which would allow this, but we're not quite there yet
What this PR does:
Refactors the frontend pipeline to accomplish a number of goals detailed below. Currently only the search pipelines have been moved over to the new structure. There will be follow up PRs for other endpoints.
Goals:
See the readme in the pipeline package for details.
Which issue(s) this PR fixes:
Fixes #1392
This PR is progress toward #3366 with an eventual goal to close it.
Changes
tempo_query_frontend_multitenant... metrics b/c they don't quite fit into the current pattern. this can be improved later.todo:
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]