Conversation
src/db/spanner/batch.rs
Outdated
| .param_types(param_types.clone()) | ||
| .execute_dml_async(&db.conn) | ||
| .await?; | ||
| db.metrics.count_with_tags( |
There was a problem hiding this comment.
this emits within the update loop (one per update) and always emits the same count_update (the number of total updates). and it's the same count emitted above (existing.len() as storage.spanner.batch.pre-existing). so I think this should just be killed?
There was a problem hiding this comment.
I should move it out of the update loop ('doh) but I was thinking more that exists is whatever was already in the table and this was the number of elements that resulted in updates, because a batch could self update as well (rare, but potential).
There was a problem hiding this comment.
If anything I was debating killing the metrics.incr above, since it's fairly low information
src/db/spanner/models.rs
Outdated
|
|
||
| pub(super) fn get_collection_name(&self, id: i32) -> String { | ||
| self.coll_cache | ||
| .get_name(id) |
There was a problem hiding this comment.
this returns <Result<Option>>
we probably wanna propagate it, there's a lock underneath which could have been poisoned. feels bad hiding that kind of error
| .get_name(id) | |
| .get_name(id)? |
There was a problem hiding this comment.
(FYI I actually implemented a _get_collection_name for mysql. apparently it is never used)
There was a problem hiding this comment.
so you're saying that it's a TODO to make this a trait? ;)
2 participants
Description
Adding a metric handler for
counts. This will allow us to do histograms to determine loads for things like batch processing.Testing
are batches metricked?
Issue(s)
Closes #878