rpc: always trace incoming RPCs if tracing is enabled #34720
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before this patch, an incoming RPC would not be traced if the caller wasn't traced. Usually this was inconsequential, as the caller is generally traced if tracing is enabled in various ways. However, for the status server (AdminUI calls) this was not true - the caller (the browser, through a HTTP->gRPC gateway) was never tracing a call. This patch makes the server create a span regardless of the caller if tracing is enabled. Fixes cockroachdb#34310 Release note: None
|
This change is |
RaduBerinde
approved these changes
Feb 12, 2019
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @andreimatei and @RaduBerinde)
pkg/rpc/context.go, line 91 at r1 (raw file):
var enableRPCCompression = envutil.EnvOrDefaultBool("COCKROACH_ENABLE_RPC_COMPRESSION", true) // spanInclusionFuncForServer is used as a SpanInclusionFunc for the server-side
[nit] since these are only used once, maybe it's easier if they're just inlined directly
andreimatei
commented
Feb 12, 2019
There was a problem hiding this comment.
bors r+
Reviewable status:
pkg/rpc/context.go, line 91 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] since these are only used once, maybe it's easier if they're just inlined directly
I thought it's better to keep them here next to each other for contrast
craig bot
pushed a commit
that referenced
this pull request
Feb 12, 2019
34301: sql: validate check constraints with the schema changer r=lucy-zhang a=lucy-zhang Currently, constraints are added in the `Unvalidated` state, and are not validated for existing rows until ALTER TABLE ... VALIDATE CONSTRAINT is run. With this change, check constraints will be validated asynchronously after they are added by default (and similar changes to FKs are to follow). This addresses the problematic long-running transactions caused by the current implementation of VALIDATE CONSTRAINT. This PR is a rework of #32504 and has the same tests. With this change, check constraints will be added to the table descriptor in the new `Validating` state, visible to CRUD operations, and a mutation is queued indicating that the constraint is to be validated. During the backfill step, the constraint is validated for existing rows. If validation succeeds, then the constraint moves to the `Validated` state; otherwise, it is dropped. The behavior when dropping constraints (either via DROP CONSTRAINT or indirectly when a column is dropped) is unchanged: no mutation is enqueued. As part of this change, check constraints can be added to non-public columns in the process of being added, including columns that were created earlier in the same transaction. The main difference between this PR and #32504 is that #32504 does not add the constraint to the table descriptor until it has been validated. See #34238 for more context. Release note (sql change): Check constraint adds by default will validate table data with the added constraint asynchronously after the transaction commits. 34720: rpc: always trace incoming RPCs if tracing is enabled r=andreimatei a=andreimatei Before this patch, an incoming RPC would not be traced if the caller wasn't traced. Usually this was inconsequential, as the caller is generally traced if tracing is enabled in various ways. However, for the status server (AdminUI calls) this was not true - the caller (the browser, through a HTTP->gRPC gateway) was never tracing a call. This patch makes the server create a span regardless of the caller if tracing is enabled. Fixes #34310 Release note: None 34798: roachtest: add two more indexes to bulk index creation test r=vivekmenezes a=vivekmenezes Release note: None 34829: jobs: only include running and very recently finished jobs in SHOW JOBS r=dt a=dt On a cluster that has been running for a long time or with frequent periodic jobs, SHOW JOBS can output an unbounded, massive wall of text. This makes it hard to find the jobs you are likely interested in -- those that are running or have very recently finished. This changes SHOW JOBS to only include running jobs or those that finished in the last 12h. The full listing of jobs is still available via crdb_internal.jobs. Release note (general change): SHOW JOBS only returns running and recently finished jobs. Older jobs can still be inspected via the crdb_internal.jobs table. Co-authored-by: Lucy Zhang <[email protected]> Co-authored-by: Andrei Matei <[email protected]> Co-authored-by: Vivek Menezes <[email protected]> Co-authored-by: David Taylor <[email protected]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this patch, an incoming RPC would not be traced if the caller
wasn't traced. Usually this was inconsequential, as the caller is
generally traced if tracing is enabled in various ways. However, for the
status server (AdminUI calls) this was not true - the caller (the
browser, through a HTTP->gRPC gateway) was never tracing a call.
This patch makes the server create a span regardless of the caller if
tracing is enabled.
Fixes #34310
Release note: None