-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
coprocessor: collect output counts for each executor #2607
Conversation
src/coprocessor/dag/dag.rs
Outdated
@@ -91,6 +93,9 @@ impl DAGContext { | |||
let mut resp = Response::new(); | |||
let mut sel_resp = SelectResponse::new(); | |||
sel_resp.set_chunks(RepeatedField::from_vec(chunks)); | |||
let mut counts = Vec::with_capacity(self.num_execs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simply use a constant number like 4 for capacity.
It doesn't worth a field in context.
LGTM |
@@ -170,6 +174,10 @@ impl Executor for IndexScanExecutor { | |||
} | |||
} | |||
|
|||
fn collect_output_counts(&mut self, counts: &mut Vec<i64>) { | |||
counts.push(self.count - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'd better set self.count to zero after collect
@@ -145,6 +148,7 @@ impl IndexScanExecutor { | |||
|
|||
impl Executor for IndexScanExecutor { | |||
fn next(&mut self) -> Result<Option<Row>> { | |||
self.count += 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'd better increase count only when meets some value since we will support stream in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM. |
/run-all-tests |
We collect it so that we could this information to adjust statistics.
PTAL @coocood @hanfei1991 @winoros @AndreMouche