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.

Sign up for GitHub

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

Jump to bottom

sql: persisted SQL Stats is not returned by tenant RPC endpoints #70585

Closed
Azhng opened this issue Sep 22, 2021 · 2 comments · Fixed by #70586
Closed

sql: persisted SQL Stats is not returned by tenant RPC endpoints #70585

Azhng opened this issue Sep 22, 2021 · 2 comments · Fixed by #70586
Assignees
@Azhng
Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker

Comments

@Azhng
Copy link
Contributor

Azhng commented Sep 22, 2021

I think this is surfacing the bigger issue where our test coverage for tenant code is a lot smaller than non-tenant code. Ideally, we only need to write once and run them in both configurations. (perhaps even better, we just have one status server used by both tenant/non-tenant). However, this is not the case today.
@Azhng Azhng added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. A-sql-observability Related to observability of the SQL layer T-sql-observability labels Sep 22, 2021
@Azhng Azhng self-assigned this Sep 22, 2021
@Azhng
Copy link
Contributor Author

Azhng commented Sep 22, 2021

This would be a great use case for adopting datadriven tests. We can one 1 line directive in our tests files to specify the type of configuration we want to run, and use our datadriven test driver to set up proper clusters.

We can even have custom directives to setup testing for RPC/HTTP endpoint tests.

@Azhng Azhng mentioned this issue Sep 22, 2021
Merged
Azhng added a commit to Azhng/cockroach that referenced this issue Sep 22, 2021
@Azhng
sql,server: enable tenant status server to work with persisted SQL stats
c2678d8 
Previsouly, tenant status server was not able to work with the new
persisted SQL Stats. This result in SQL Stats RPC not able to return
or reset persisted SQL stats.
This commit addresses this issue to update tenant status server's
implementation to be able to work with persisted SQL Stats.

Resolves cockroachdb#70585 cockroachdb#70529 cockroachdb#68177

Release Justification: Bug fixes and low-risk updates to new functionality

Release note (bug fix): Statement/Transaction Page now is able to
display and reset persisted SQL Stats.
Azhng added a commit to Azhng/cockroach that referenced this issue Sep 23, 2021
@Azhng
sql,server: enable tenant status server to work with persisted SQL stats
38098fa 
Previsouly, tenant status server was not able to work with the new
persisted SQL Stats. This result in SQL Stats RPC not able to return
or reset persisted SQL stats.
This commit addresses this issue to update tenant status server's
implementation to be able to work with persisted SQL Stats.

Resolves cockroachdb#70585 cockroachdb#70529 cockroachdb#68177

Release Justification: Bug fixes and low-risk updates to new functionality

Release note (bug fix): Statement/Transaction Page now is able to
display and reset persisted SQL Stats.
@maryliag maryliag added GA-blocker and removed release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Sep 23, 2021
Azhng added a commit to Azhng/cockroach that referenced this issue Sep 23, 2021
@Azhng
sql,server: enable tenant status server to work with persisted SQL stats
7d0e182 
Previsouly, tenant status server was not able to work with the new
persisted SQL Stats. This result in SQL Stats RPC not able to return
or reset persisted SQL stats.
This commit addresses this issue to update tenant status server's
implementation to be able to work with persisted SQL Stats.

Resolves cockroachdb#70585 cockroachdb#70529 cockroachdb#68177

Release Justification: Bug fixes and low-risk updates to new functionality

Release note (bug fix): Statement/Transaction Page now is able to
display and reset persisted SQL Stats.
Azhng added a commit to Azhng/cockroach that referenced this issue Sep 23, 2021
@Azhng
sql,server: enable tenant status server to work with persisted SQL stats
09993f4 
Previsouly, tenant status server was not able to work with the new
persisted SQL Stats. This result in SQL Stats RPC not able to return
or reset persisted SQL stats.
This commit addresses this issue to update tenant status server's
implementation to be able to work with persisted SQL Stats.

Resolves cockroachdb#70585 cockroachdb#70529 cockroachdb#68177

Release Justification: Bug fixes and low-risk updates to new functionality

Release note (bug fix): Statement/Transaction Page now is able to
display and reset persisted SQL Stats.
Azhng added a commit to Azhng/cockroach that referenced this issue Sep 24, 2021
@Azhng
sql,server: enable tenant status server to work with persisted SQL stats
4a220ab 
Previously, tenant status server was not able to work with the new
persisted SQL Stats. This result in SQL Stats RPC not able to return
or reset persisted SQL stats.
This commit addresses this issue by:
* updating tenant status server's Statements endpoint to be able to work
  with persisted SQL Stats.
* introducing additional parameter in ResetSQLStatsRequest to specify
  whether persisted SQL Stats need to be reset.

Resolves cockroachdb#70585 cockroachdb#70529 cockroachdb#68177

Release note (bug fix): Statement/Transaction Page now is able to
display and reset persisted SQL Stats.
craig bot pushed a commit that referenced this issue Sep 25, 2021
@Azhng @sumeerbhola
Merge #70586 #70707
9ba8499 
70586: sql,server: enable tenant status server to work with persisted SQL stats r=maryliag,dhartunian a=Azhng

Previsouly, tenant status server was not able to work with the new
persisted SQL Stats. This result in SQL Stats RPC not able to return
or reset persisted SQL stats.
This commit addresses this issue to update tenant status server's
implementation to be able to work with persisted SQL Stats.

Resolves #70585 #70529 #68177

Release Justification: Bug fixes and low-risk updates to new functionality

Release note (bug fix): Statement/Transaction Page now is able to
display and reset persisted SQL Stats.

70707: clusterversion,storage: cluster version that uses Pebble's SetWithDelete r=sumeerbhola a=sumeerbhola

This is a backwards incompatible change in Pebble to provide
SingleDelete semantics that are clean and robust to programming
error.

This is being done post v21.2-beta but before GA since this
was deemed a GA blocker and not a release blocker. It allows
for upgrading beta clusters to the GA version. For more on these
discussions see the following threads
cockroachdb/pebble#1255 (comment),
https://cockroachlabs.slack.com/archives/C4A9ALLRL/p1631213490022600
(Cockroach Labs internal link).

Informs cockroachdb/pebble#1255
Informs #69891

Release note: None

Release justification: high-severity bug fix

Co-authored-by: Azhng <archer.xn@gmail.com>
Co-authored-by: sumeerbhola <sumeer@cockroachlabs.com>
@craig craig bot closed this as completed in 92e1967 Sep 25, 2021
@Azhng
Copy link
Contributor Author

Azhng commented Sep 25, 2021

Reopen for backporting

@Azhng Azhng reopened this Sep 25, 2021
maryliag pushed a commit to maryliag/cockroach that referenced this issue Sep 27, 2021
@Azhng @maryliag
sql,server: enable tenant status server to work with persisted SQL stats
8bfdeb6 
Previously, tenant status server was not able to work with the new
persisted SQL Stats. This result in SQL Stats RPC not able to return
or reset persisted SQL stats.
This commit addresses this issue by:
* updating tenant status server's Statements endpoint to be able to work
  with persisted SQL Stats.
* introducing additional parameter in ResetSQLStatsRequest to specify
  whether persisted SQL Stats need to be reset.

Resolves cockroachdb#70585 cockroachdb#70529 cockroachdb#68177

Release note (bug fix): Statement/Transaction Page now is able to
display and reset persisted SQL Stats.
@tbg tbg mentioned this issue Oct 7, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Azhng Azhng

Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql,server: enable tenant status server to work with persisted SQL stats Azhng/cockroach
2 participants
@maryliag @Azhng