recording the cost for different types of queries #1829
Conversation
search/util.go
Outdated
| type SearchIOStatsCallbackFunc func(uint64) | ||
|
|
||
| // The callback signature is (message, queryType, cost) which allows | ||
| // the caller to act on a particular query type and what its the associated |
There was a problem hiding this comment.
Can we concise "query type and what its the associated cost of an operation."
to
"query type and it's associated cost of operation"
search/util.go
Outdated
|
|
||
| // The callback signature is (message, queryType, cost) which allows | ||
| // the caller to act on a particular query type and what its the associated | ||
| // cost of an operation. "add" indicates to increment the cost for the query |
There was a problem hiding this comment.
Values of message argument (like "add" and "done" ) aren't defined by bleve, it is upto the user of bleve.
If you have specified "add" and "done" as examples of message argument value, then we can frame it that way
There was a problem hiding this comment.
hmm, the cost callback according to me includes very basic set of messages, which are add, done and maybe abort, since its mainly enabling the caller to track the costs incrementally and act upon that along with a "done" type of a message that will help the caller to act upon the tracked bytes until now. perhaps an abort invocation can be done when there is a ctx.Done() in the collector phase (which would happen when the search's context is cancelled i believe) in that case, the caller can handle separately perhaps.
There was a problem hiding this comment.
Got your point.
Bleve will invoke the callback with these messages "add", "done", "abort" etc ...
And it is the responsibility of the author of the callback to properly handle these messages.
In this case, we can frame our comment like this:
"Implementation of SearchIncrementalCostCallbackFn should handle the following messages
- add: "description ..."
- abort: description ...
- done: description ...
"
We can even assign IDs to these messages and export from bleve
type IncCostMessage uint
const (
AddM = Service(1 << iota) // description
AbortM // description
DoneM // description
)
type SearchIncrementalCostCallbackFn func(IncCostMessage, string, uint64)
Thoughts ?
There was a problem hiding this comment.
yeah i've renamed the messages and query types, thanks
| @@ -486,14 +486,14 @@ func (ss *SearchStatus) Merge(other *SearchStatus) { | |||
| // A SearchResult describes the results of executing | |||
| // a SearchRequest. | |||
| type SearchResult struct { | |||
There was a problem hiding this comment.
Would you add commentary above this construct explaining what each of the response attribute stands for. This would then include what our definition for Cost is - the user can benefit from this with godocs.
search.go
Outdated
| Request *SearchRequest `json:"request"` | ||
| Hits search.DocumentMatchCollection `json:"hits"` | ||
| Total uint64 `json:"total_hits"` | ||
| Cost uint64 `json:"search_cost"` |
There was a problem hiding this comment.
Perhaps the json bit should simply be cost - given that this is already part of SearchResult.
index_impl.go
Outdated
| totalBytesRead += storedFieldsBytes | ||
| totalSearchCost += storedFieldsBytes | ||
|
|
||
| search.RecordSearchCost(ctx, search.AddM, storedFieldsBytes) |
There was a problem hiding this comment.
Make this one call as opposed to per hit.
- The main approach used over here is leveraging the callbacks that can be set in the context over which the search is performed. - Bleve side of things invokes this callback whenever it reaches a point where the bytes have to be converted and metered against regulator. - blevesearch/bleve#1829 - Also, removing the old APIs for the metering along read paths, as we are now using callbacks. Change-Id: I500e87f1e5ea4ffbf0ec254c8fc36ece0328a904 Reviewed-on: https://review.couchbase.org/c/cbft/+/191695 Well-Formed: Build Bot <[email protected]> Tested-by: <[email protected]> Reviewed-by: <[email protected]>
Adds a new callback function SearchIncrementalCostCallbackFn which is used to track incremental cost updates for the query. It can also be used to track costs for different types of queries as well.
Renames "bytesRead" to "search_cost"