Skip to content
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

sql: identify the queuing time separately in query stats #22896

Closed
knz opened this issue Feb 21, 2018 · 6 comments
Closed

sql: identify the queuing time separately in query stats #22896

knz opened this issue Feb 21, 2018 · 6 comments
Labels
A-sql-execution Relating to SQL execution. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@knz
Copy link
Contributor

knz commented Feb 21, 2018

Prior to #22277 there was no server-side input queue so queries would get executed as soon as they were reused. With #22277 now a query can stay in a queue server-side for a little while before it gets processed.

This waiting time can and should be measured and reported.

(Requested by @andreimatei )

@knz knz added this to the 2.0 milestone Feb 21, 2018
@knz knz self-assigned this Feb 21, 2018
@knz knz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Feb 27, 2018
@jordanlewis jordanlewis modified the milestones: 2.0, 2.1 Mar 13, 2018
@knz knz added the A-sql-execution Relating to SQL execution. label May 14, 2018
@knz knz removed their assignment May 14, 2018
@jordanlewis jordanlewis modified the milestones: 2.1, 2.2 Aug 30, 2018
@petermattis petermattis removed this from the 2.2 milestone Oct 5, 2018
@jordanlewis
Copy link
Member

@andreimatei where is this input queue? Do you want to do this for us? Seems like you could probably knock it out more quickly than others.

@andreimatei
Copy link
Contributor

The "input queue" is the StmtBuf - that's where queries (and generally, commands) sit in between being read from the network (by pgwire.conn) and being processed (by connExecutor).
Realistically, I can't take care of this issue; my stack is way to deep at the moment, sorry.
I think there's a bit more to the context then "requested by @andreimatei" would let you believe. If memory doesn't fail me, I was arguing that we're measuring too many latencies that I didn't think they were valuable and then I used this as an example of something we don't measure. So I think I was arguing against some other measurements, not for more of them. However, I don't think this issue is necessarily invalid. I'm basically washing my hands :).

@jordanlewis
Copy link
Member

Any clues on what the "too many measurements" part was about?

@andreimatei
Copy link
Contributor

Maybe parsing time... although now I wouldn't argue against that one. I don't remember well; I think there was some measurements structure that was in my way when refactoring the Executor that I was hating on.

@github-actions
Copy link

github-actions bot commented Jun 7, 2021

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
5 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@jordanlewis
Copy link
Member

We don't think this is necessary to track for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-execution Relating to SQL execution. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

No branches or pull requests

4 participants