This project is mirrored from https://github.com/cockroachdb/cockroach. Pull mirroring updated .
  1. 10 Jul, 2020 2 commits
    • craig[bot]'s avatar
      Merge #50867 · bd6a7bda
      craig[bot] authored
      50867: colexec: simplify DrainMeta and Close implementations r=yuzefovich a=asubiotto
      
      Since DrainMeta and Close are now called from the same goroutine as Next, the implementations can be simplified. This PR does that in a couple of commits and fixes a bug in the first commit.
      
      Release note: None
      Co-authored-by: default avatarAlfonso Subiotto Marques <[email protected]>
      bd6a7bda
    • craig[bot]'s avatar
      Merge #50647 · 0ac546b5
      craig[bot] authored
      50647: sqlproxy: initial commit and end-to-end demo r=asubiotto a=tbg
      
      This adds the proxy envisioned for use with multi-tenant deployments.
      Initially, the proxy routes all incoming connections to a backend SQL
      server indicated via a command-line flag.
      
      This is good enough for initial sanity checks, but the following changes
      need to be made:
      
      - allow `--insecure` (for testing purposes only), which means that some
        of the steps in the proxy are skipped
      - add an in-memory Proxy() to (TestServer).StartTenant as well as the
        3node-tenant logic test configuration.
      - move `pkg/cmd/mtproxy` to `./cockroach mt start-proxy`. In doing so,
        the proxy will get proper certificate management (the certs types
        will have to be added though), which makes it easier to use.
      - implement the parsing of tenant identifier from the database name or
         options field
      - load a tenant identifier -> tenant ID map from a file specified via a
          command-line flag
      - reload that map on SIGHUP
      - unit testing of the resulting code used in the `./cockroach mt
        start-proxy` command (there is already some testing of the `sqlproxy`
        package itself)
      
      The following snippet sets up a demo SQL shell connecting, securely,
      through the proxy to the SQL tenant server, which in turn talks to
      the KV layer.
      
          #!/bin/bash
      
          set -euxo pipefail
      
          # Prep work
          rm -rf ~/.cockroach-certs cockroach-data
          killall -9 cockroach || true
          killall -9 mtproxy || true
      
          export CERTSDIR=$HOME/.cockroach-certs
          export COCKROACH_CA_KEY=$CERTSDIR/ca.key
          ./cockroach cert create-ca
          ./cockroach cert create-client root
          ./cockroach cert create-node 127.0.0.1 localhost
      
          #  Start KV layer. (:26257)
          ./cockroach start --host 127.0.0.1 --background
          # Create tenant
          ./cockroach sql --host 127.0.0.1 -e 'select crdb_internal.create_tenant(123);'
      
          # Spawn tenant SQL server (:36257) pointing at KV layer (:26257)
          ./cockroach mt start-sql --tenant-id 123 --kv-addrs 127.0.0.1:26257 --sql-addr 127.0.0.1:36257 &
          sleep 1
      
          # Create user on tenant (proxy does not forward client certs)
          ./cockroach sql --port 36257 -e "create user bob with password 'builder';"
      
          # Spawn the proxy on :5432, forwarding to tenant SQL server (:36257)
          go run ./pkg/cmd/mtproxy/ -cert-file $CERTSDIR/node.crt -key-file $CERTSDIR/node.key -verify=false --target 127.0.0.1:36257 &
      
          sleep 5
      
          # Connect to proxy.
          ./cockroach sql --host 127.0.0.1 --port 5432 --user bob
      
      cc @imjching 
      
      Release note: None
      Co-authored-by: default avatarTobias Schottdorf <[email protected]>
      0ac546b5
  2. 09 Jul, 2020 38 commits
    • craig[bot]'s avatar
      Merge #50909 · 542c83e7
      craig[bot] authored
      50909: sql: add sort and virtual scan support in the new factory r=yuzefovich a=yuzefovich
      
      **sql: implement ConstructSort in the new factory**
      
      Release note: None
      
      **sql: add support of virtual scans in the new factory**
      
      The support is added by sharing the same code with the old factory that
      construct `delayedNode` with the rest of necessary methods already being
      implemented in the new factory. That delayedNode is wrapped into the
      physical plan via a callback.
      
      Note that with the new factory it is possible to have a distributed plan
      that contains an execinfra.LocalProcessor. This required to make
      a change that distsql.LocalState, regardless of the plan distribution,
      has always LocalProcs set when setting up a flow on the gateway.
      
      Addresses: #47473.
      
      Release note: None
      Co-authored-by: default avatarYahor Yuzefovich <[email protected]>
      542c83e7
    • craig[bot]'s avatar
      Merge #51235 · 753fe8f5
      craig[bot] authored
      51235: colfetcher: assert that the colfetcher is not being used for SCRUB r=andreimatei a=andreimatei
      
      The colfetch ctor took an unused `check` arg, which is now removed.
      That check comes from a TableReader spec, in particular one for a SCRUB
      (I think). We now assert that a colfetcher is not created for such a
      spec - hopefully there's something preventing it. Were it to be created,
      I'm pretty sure the SCRUB was not getting what it wanted.
      
      Release note: None
      Co-authored-by: default avatarAndrei Matei <[email protected]>
      753fe8f5
    • craig[bot]'s avatar
      Merge #51178 · 65a30940
      craig[bot] authored
      51178: sql/rowexec: delete indexSkipTableReader r=andreimatei a=andreimatei
      
      This processor is not used. It was implemented for #24584, but the PR to
      start planning it stalled since Ridwan left, a year ago (#39668).
      This guy is marginally in my way, and, worse, it's dead code. Unless
      someone promises to finish that PR imminently :).
      
      Release note: None
      Co-authored-by: default avatarAndrei Matei <[email protected]>
      65a30940
    • Yahor Yuzefovich's avatar
      sql: add support of virtual scans in the new factory · 3ee7694c
      Yahor Yuzefovich authored
      The support is added by sharing the same code with the old factory that
      construct `delayedNode` with the rest of necessary methods already being
      implemented in the new factory. That delayedNode is wrapped into the
      physical plan via a callback.
      
      Note that with the new factory it is possible to have a distributed plan
      that contains an execinfra.LocalProcessor. This required to make
      a change that distsql.LocalState, regardless of the plan distribution,
      has always LocalProcs set when setting up a flow on the gateway.
      
      Release note: None
      3ee7694c
    • Tobias Schottdorf's avatar
      sqlproxy: initial commit and end-to-end demo · e71886ab
      Tobias Schottdorf authored
      This adds the proxy envisioned for use with multi-tenant deployments.
      Initially, the proxy routes all incoming connections to a backend SQL
      server indicated via a command-line flag.
      
      This is good enough for initial sanity checks, but the following changes
      need to be made:
      
      - allow `--insecure` (for testing purposes only), which means that some
        of the steps in the proxy are skipped
      - add an in-memory Proxy() to (TestServer).StartTenant as well as the
        3node-tenant logic test configuration.
      - move `pkg/cmd/mtproxy` to `./cockroach mt start-proxy`. In doing so,
        the proxy will get proper certificate management (the certs types
        will have to be added though), which makes it easier to use.
      - implement the parsing of tenant identifier from the database name or
         options field
      - load a tenant identifier -> tenant ID map from a file specified via a
          command-line flag
      - reload that map on SIGHUP
      - unit testing of the resulting code used in the `./cockroach mt
        start-proxy` command (there is already some testing of the `sqlproxy`
        package itself)
      
      The following snippet sets up a demo SQL shell connecting, securely,
      through the proxy to the SQL tenant server, which in turn talks to
      the KV layer.
      
          #!/bin/bash
      
          set -euxo pipefail
      
          # Prep work
          rm -rf ~/.cockroach-certs cockroach-data
          killall -9 cockroach || true
          killall -9 mtproxy || true
      
          export CERTSDIR=$HOME/.cockroach-certs
          export COCKROACH_CA_KEY=$CERTSDIR/ca.key
          ./cockroach cert create-ca
          ./cockroach cert create-client root
          ./cockroach cert create-node 127.0.0.1 localhost
      
          #  Start KV layer. (:26257)
          ./cockroach start --host 127.0.0.1 --background
          # Create tenant
          ./cockroach sql --host 127.0.0.1 -e 'select crdb_internal.create_tenant(123);'
      
          # Spawn tenant SQL server (:36257) pointing at KV layer (:26257)
          ./cockroach mt start-sql --tenant-id 123 --kv-addrs 127.0.0.1:26257 --sql-addr 127.0.0.1:36257 &
          sleep 1
      
          # Create user on tenant (proxy does not forward client certs)
          ./cockroach sql --port 36257 -e "create user bob with password 'builder';"
      
          # Spawn the proxy on :5432, forwarding to tenant SQL server (:36257)
          go run ./pkg/cmd/mtproxy/ -cert-file $CERTSDIR/node.crt -key-file $CERTSDIR/node.key -verify=false --target 127.0.0.1:36257 &
      
          sleep 5
      
          # Connect to proxy.
          ./cockroach sql --host 127.0.0.1 --port 5432 --user bob
      
      Release note: None
      e71886ab
    • craig[bot]'s avatar
      Merge #50639 · 014a94f2
      craig[bot] authored
      50639: metrics/aggmetric,kvserver/tenantrate: add rate limiting and monitoring for tenants r=tbg a=ajwerner
      
      See individual commits. 
      
      Touches #47905, #47906.
      Co-authored-by: default avatarAndrew Werner <[email protected]>
      014a94f2
    • Yahor Yuzefovich's avatar
      sql: implement ConstructSort in the new factory · 1b58e6bc
      Yahor Yuzefovich authored
      Release note: None
      1b58e6bc
    • craig[bot]'s avatar
      Merge #51196 · abfe5d12
      craig[bot] authored
      51196: roachtest: fix parsing of django test results r=rafiss a=rafiss
      
      Use a custom test runner class to avoid printing out descriptions of
      test cases, which make it impossible to parse test results in some
      cases.
      
      Also, update the parsing logic to look for "expected failure" and
      "unexpected success" and handle these results appropriately.
      
      Release note: None
      Co-authored-by: default avatarRafi Shamim <[email protected]>
      abfe5d12
    • Andrei Matei's avatar
      colfetcher: assert that the colfetcher is not being used for SCRUB · 564e24d3
      Andrei Matei authored
      The colfetch ctor took an unused `check` arg, which is now removed.
      That check comes from a TableReader spec, in particular one for a SCRUB
      (I think). We now assert that a colfetcher is not created for such a
      spec - hopefully there's something preventing it. Were it to be created,
      I'm pretty sure the SCRUB was not getting what it wanted.
      
      Release note: None
      564e24d3
    • craig[bot]'s avatar
      Merge #51105 · 1bc2b26e
      craig[bot] authored
      51105: sql: stop modifying table descriptors during type hydration r=ajwerner a=rohany
      
      We were incorrectly modifying `ImmutableTableDescriptors` when hydrating
      the type metadata within them. This PR changes the behavior to make a
      copy of the `ImmutableTableDescriptor` before performing the hydration
      logic. Note that this is not the end goal of this process -- we are
      working towards maintaining caches of table descriptors with hydrated
      type metadata.
      
      Release note: None
      Co-authored-by: default avatarRohan Yadav <[email protected]>
      1bc2b26e
    • craig[bot]'s avatar
      Merge #51164 · e84d0cbe
      craig[bot] authored
      51164: opt: build partial index predicates as FiltersExprs r=RaduBerinde a=mgartner
      
      This commit updates optbuilder to build a partial index predicate as
      `FiltersExpr` instead of an arbitrary ScalarExpr. The primary reason for
      doing so is to make it easier to calculate statistics for partial index
      scans. By representing the predicate as a `FiltersExpr`, the statistics
      builder can generate stats for partial index scans using existing
      functions that help generate statistics for Selects.
      
      `SimplifyFilters` and `ConsolidateFilters` are manually called on the
      predicate so that it is similar in form to a normalized Select filter.
      Flattening the expression and generating constraints for each top-level
      conjunction will make it simpler and cheaper to calculate stats
      accurately for unapplied conjunctions.
      
      This commit also updates `Implicator` to handle predicates that are
      `FiltersExpr`s and have `RangeExpr`s in them. This change benefits the
      performance of `Implicator` significantly in some of the slowest
      benchmark cases because flattening predicate conjunctions means that the
      exact-match fast path can prove implication in more cases. This change
      only worsens performance significantly in some of the fastest cases that
      take <400ns/op.
      
          name                                               old time/op  new time/op  delta
          Implicator/single-exact-match-16                   61.7ns ± 0%  89.1ns ± 0%  +44.47%  (p=0.008 n=5+5)
          Implicator/single-inexact-match-16                  878ns ± 0%   912ns ± 1%   +3.94%  (p=0.008 n=5+5)
          Implicator/range-inexact-match-16                  2.32µs ± 2%  2.40µs ± 1%   +3.78%  (p=0.008 n=5+5)
          Implicator/single-exact-match-extra-filters-16      321ns ± 1%   371ns ± 1%  +15.70%  (p=0.008 n=5+5)
          Implicator/single-inexact-match-extra-filters-16   4.00µs ± 0%  4.11µs ± 1%   +2.78%  (p=0.008 n=5+5)
          Implicator/multi-column-and-exact-match-16          455ns ± 0%   100ns ± 1%  -78.02%  (p=0.016 n=4+5)
          Implicator/multi-column-and-inexact-match-16       1.94µs ± 1%  1.97µs ± 0%   +1.32%  (p=0.040 n=5+5)
          Implicator/multi-column-or-exact-match-16          62.1ns ± 0%  91.4ns ± 0%  +47.23%  (p=0.008 n=5+5)
          Implicator/multi-column-or-exact-match-reverse-16  1.72µs ± 0%  1.79µs ± 0%   +4.08%  (p=0.008 n=5+5)
          Implicator/multi-column-or-inexact-match-16        2.18µs ± 0%  2.25µs ± 1%   +3.56%  (p=0.008 n=5+5)
          Implicator/and-filters-do-not-imply-pred-16        3.65µs ± 2%  3.76µs ± 5%     ~     (p=0.056 n=5+5)
          Implicator/or-filters-do-not-imply-pred-16          774ns ± 1%   850ns ± 1%   +9.71%  (p=0.008 n=5+5)
          Implicator/many-columns-exact-match10-16           4.93µs ± 1%  0.30µs ± 1%  -93.87%  (p=0.008 n=5+5)
          Implicator/many-columns-inexact-match10-16         11.3µs ± 1%  11.1µs ± 0%   -1.86%  (p=0.008 n=5+5)
          Implicator/many-columns-exact-match100-16           352µs ± 1%    18µs ± 1%  -94.92%  (p=0.008 n=5+5)
          Implicator/many-columns-inexact-match100-16         382µs ± 1%   372µs ± 1%   -2.47%  (p=0.008 n=5+5)
      
      Release note: None
      Co-authored-by: default avatarMarcus Gartner <[email protected]>
      e84d0cbe
    • craig[bot]'s avatar
      Merge #50924 #51065 #51121 #51163 #51194 #51218 #51226 · ba2deacf
      craig[bot] authored
      50924: cli: add commands for managing statement diagnostics r=RaduBerinde a=RaduBerinde
      
      This change adds a `statement-diag` command, with the following subcommands:
      ```
        list        list available bundles and outstanding activation requests
        download    download statement diagnostics bundle into a zip file
        delete      delete a statement diagnostics bundle
        cancel      cancel an outstanding activation request
      ```
      
      Fixes #48597.
      
      Release note (cli change): A new set of `statement-diag` CLI commands that can
      be used to manage statement diagnostics.
      
      51065: importccl: fix target column ordering bug for PGDUMP import r=mjibson a=Anzoteh96
      
      The current implementation assumes that the target columns of a PGDUMP query is the same as how they are created in the case where target columns are declared in PGDUMP file. This PR addresses it by detecting the target columns in the PGDUMP statement itself if this is the case. In addition, given that the target columns may not be well-determined at the formation of a new `DatumRowConverter`, so the check of unsupported default column expression is also moved to the `DatumRowConverter.Row()` function. 
      
      Fixed #51159 
      
      Release note: None
      
      51121: sql: move parallelize scans control in the execbuilder r=RaduBerinde a=RaduBerinde
      
      Parallel scans refers to disabling scan batch limits, which allows the
      distsender to issue requests to multiple ranges in parallel. This is
      only safe to use when there is a known upper bound for the number of
      results.
      
      Currently we plumb maxResults to the scanNode and TableReader, and
      each execution component runs similar logic to decide whether to
      parallelize.
      
      This change cleans this up by centralizing this decision inside the
      execbuilder. In the future, we may want the coster to be aware of this
      parallelization as well.
      
      For simplicity, we drop the cluster setting that controls this (it was
      added for fear of problems but it has been on by default for a long
      time).
      
      Release note: None
      
      51163: jobs: do not include cancel jobs as running for scheduled jobs r=pbardea a=pbardea
      
      Previously, the query that the job scheduling system would use to detect
      if there were any already running jobs would include canceled jobs due
      to using a the alternative spelling: 'cancelled'.
      
      This commit changes the query to use the enums instead of manually
      listing their state. It also adds a test to ensure that jobs are still
      run, regardless of the wait policy when all previous runs are in a
      terminal state.
      
      Release note: None
      
      51194: pgwire: de-strictify extended protocol handling r=jordanlewis a=jordanlewis
      
      Fixes #33693.
      Fixes #41511.
      
      Previously, the protocol handler didn't permit simple queries during the
      extended protocol mode. I think this was done because the spec vaguely
      says that extended protocol commands must be ended with a SYNC command:
      https://www.postgresql.org/docs/9.3/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY
      
      However, an examination of the PostgreSQL source code shows that the
      extended protocol mode bit is only used for error recovery. If an error
      is encountered during extended protocol mode in Postgres, all commands
      are skipped until the next Sync.
      
      CockroachDB never implemented that behavior - instead, it used the
      extended protocol mode bit only to reject simple queries if they came
      before the Sync.
      
      This commit removes the extended protocol mode bit, as the use case we
      used it for was incorrect. It's unclear to me whether we need to re-add
      the bit for dealing with error cases, but we haven't needed it yet.
      Adding that might be follow-on work, and won't come in this PR.
      
      Release note (bug fix): prevent spurious "SimpleQuery not allowed while
      in extended protocol mode" errors.
      
      51218: Fix typo in txn.commits metric help text r=nvanbenschoten a=a-robinson
      
      Sorry for the mostly useless change, but it bothered me :)
      
      51226: opt: fix error in case of casted NULL arguments to AddGeometryColumn r=rytaft a=rytaft
      
      This commit fixes an error that occurred when `AddGeometryColumn` was
      called with `NULL` arguments that were cast to the type specified by the
      function signature. #50992 already fixed the case when `AddGeometryColumn`
      was called with bare `NULL` arguments, since those were detected by `TypeCheck`.
      `TypeCheck` does not detect `NULL` arguments if they are cast to the correct
      type.
      
      This commit fixes the error by adding an explicit check in the `optbuilder`
      that each argument is not null before calling the `SQLFn` of the
      `AddGeometryColumn` overload.
      
      Informs #50296
      
      Release note (bug fix): Fixed an internal error that occurred when
      AddGeometryColumn was called with NULL arguments. This now results in
      a no-op and returns NULL.
      Co-authored-by: default avatarRadu Berinde <[email protected]>
      Co-authored-by: default avataranzoteh96 <[email protected]>
      Co-authored-by: default avatarPaul Bardea <[email protected]>
      Co-authored-by: default avatarJordan Lewis <[email protected]>
      Co-authored-by: default avatarAlex Robinson <[email protected]>
      Co-authored-by: default avatarRebecca Taft <[email protected]>
      ba2deacf
    • Marcus Gartner's avatar
      opt: build partial index predicates as FiltersExprs · be87984d
      Marcus Gartner authored
      This commit updates optbuilder to build a partial index predicate as
      `FiltersExpr` instead of an arbitrary ScalarExpr. The primary reason for
      doing so is to make it easier to calculate statistics for partial index
      scans. By representing the predicate as a `FiltersExpr`, the statistics
      builder can generate stats for partial index scans using existing
      functions that help generate statistics for Selects.
      
      `SimplifyFilters` and `ConsolidateFilters` are manually called on the
      predicate so that it is similar in form to a normalized Select filter.
      Flattening the expression and generating constraints for each top-level
      conjunction will make it simpler and cheaper to calculate stats
      accurately for unapplied conjunctions.
      
      This commit also updates `Implicator` to handle predicates that are
      `FiltersExpr`s and have `RangeExpr`s in them. This change benefits the
      performance of `Implicator` significantly in some of the slowest
      benchmark cases because flattening predicate conjunctions means that the
      exact-match fast path can prove implication in more cases. This change
      only worsens performance significantly in some of the fastest cases that
      take <400ns/op.
      
          name                                               old time/op  new time/op  delta
          Implicator/single-exact-match-16                   61.7ns ± 0%  89.1ns ± 0%  +44.47%  (p=0.008 n=5+5)
          Implicator/single-inexact-match-16                  878ns ± 0%   912ns ± 1%   +3.94%  (p=0.008 n=5+5)
          Implicator/range-inexact-match-16                  2.32µs ± 2%  2.40µs ± 1%   +3.78%  (p=0.008 n=5+5)
          Implicator/single-exact-match-extra-filters-16      321ns ± 1%   371ns ± 1%  +15.70%  (p=0.008 n=5+5)
          Implicator/single-inexact-match-extra-filters-16   4.00µs ± 0%  4.11µs ± 1%   +2.78%  (p=0.008 n=5+5)
          Implicator/multi-column-and-exact-match-16          455ns ± 0%   100ns ± 1%  -78.02%  (p=0.016 n=4+5)
          Implicator/multi-column-and-inexact-match-16       1.94µs ± 1%  1.97µs ± 0%   +1.32%  (p=0.040 n=5+5)
          Implicator/multi-column-or-exact-match-16          62.1ns ± 0%  91.4ns ± 0%  +47.23%  (p=0.008 n=5+5)
          Implicator/multi-column-or-exact-match-reverse-16  1.72µs ± 0%  1.79µs ± 0%   +4.08%  (p=0.008 n=5+5)
          Implicator/multi-column-or-inexact-match-16        2.18µs ± 0%  2.25µs ± 1%   +3.56%  (p=0.008 n=5+5)
          Implicator/and-filters-do-not-imply-pred-16        3.65µs ± 2%  3.76µs ± 5%     ~     (p=0.056 n=5+5)
          Implicator/or-filters-do-not-imply-pred-16          774ns ± 1%   850ns ± 1%   +9.71%  (p=0.008 n=5+5)
          Implicator/many-columns-exact-match10-16           4.93µs ± 1%  0.30µs ± 1%  -93.87%  (p=0.008 n=5+5)
          Implicator/many-columns-inexact-match10-16         11.3µs ± 1%  11.1µs ± 0%   -1.86%  (p=0.008 n=5+5)
          Implicator/many-columns-exact-match100-16           352µs ± 1%    18µs ± 1%  -94.92%  (p=0.008 n=5+5)
          Implicator/many-columns-inexact-match100-16         382µs ± 1%   372µs ± 1%   -2.47%  (p=0.008 n=5+5)
      
      Release note: None
      be87984d
    • Andrew Werner's avatar
      quotapool,tenantrate: provide mechanism to avoid notifying in Add · d6d6a7d3
      Andrew Werner authored
      It is not great to wake the head of the queue when removing resources from the
      quotapool.
      
      Release note: None
      d6d6a7d3
    • Andrew Werner's avatar
      kvserver/tenantrate: add a package to rate limit tenants · 0e278382
      Andrew Werner authored
      The basic concept is that the `*Store` will hold on to a `*Factory` and a
      `*Replica` will hold on to a reference-counted `*Limiter`. The Limiter is
      controlled through cluster settings. The Limiter in its initial conception
      controls rate and burst settings for requests, bytes read, and bytes written.
      
      We cannot know how many bytes will be read a priori so instead we acquire a
      single byte for each request and then subtract out the number of bytes read
      after a request completes. This approach may put the token bucket into debt,
      blocking future requests from beginning.
      
      The current thinking is that a Replica will get its rate limiter during
      `Replica.setDesc()` by checking to see if the `StartKey` and `EndKey` imply
      a tenant ID.
      
      Release note: None
      0e278382
    • Andrew Werner's avatar
      settings: add RegisterPositiveFloatSetting · d36ee5b4
      Andrew Werner authored
      Release note: None
      d36ee5b4
    • Andrew Werner's avatar
      server/status: add support for aggmetric in the status recorder · 2583db8e
      Andrew Werner authored
      Release note: None
      2583db8e
    • Andrew Werner's avatar
      util/metric/aggmetric: provide package to build aggregated metrics · ce963b3e
      Andrew Werner authored
      This commit adds a new metric subpackage with wrappers around existing metric
      types which enable keeping metrics for tenants. The concept here is that we'll
      create metrics for each tenant as the child of some metric. For metric
      collection internally, we'll track the aggregate but for prometheus we'll
      export all of the tenants too with added labels.
      
      This package exposes a general concept of an aggregate metric which has
      children. The individual children are directly manipulated and their values
      are exposed to the prometheus endpoint but not to the internal cockroach
      timeseries database.
      
      Release note: None
      ce963b3e
    • Radu Berinde's avatar
      sql: move parallelize scans control in the execbuilder · 256ba771
      Radu Berinde authored
      Parallel scans refers to disabling scan batch limits, which allows the
      distsender to issue requests to multiple ranges in parallel. This is
      only safe to use when there is a known upper bound for the number of
      results.
      
      Currently we plumb maxResults to the scanNode and TableReader, and
      each execution component runs similar logic to decide whether to
      parallelize.
      
      This change cleans this up by centralizing this decision inside the
      execbuilder. In the future, we may want the coster to be aware of this
      parallelization as well.
      
      For simplicity, we drop the cluster setting that controls this (it was
      added for fear of problems but it has been on by default for a long
      time).
      
      Release note: None
      256ba771
    • Jordan Lewis's avatar
      pgwire: de-strictify extended protocol handling · a6593138
      Jordan Lewis authored
      Previously, the protocol handler didn't permit simple queries during the
      extended protocol mode. I think this was done because the spec vaguely
      says that extended protocol commands must be ended with a SYNC command:
      https://www.postgresql.org/docs/9.3/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY
      
      However, an examination of the PostgreSQL source code shows that the
      extended protocol mode bit is only used for error recovery. If an error
      is encountered during extended protocol mode in Postgres, all commands
      are skipped until the next Sync.
      
      CockroachDB never implemented that behavior - instead, it used the
      extended protocol mode bit only to reject simple queries if they came
      before the Sync.
      
      This commit removes the extended protocol mode bit, as the use case we
      used it for was incorrect. It's unclear to me whether we need to re-add
      the bit for dealing with error cases, but we haven't needed it yet.
      Adding that might be follow-on work, and won't come in this PR.
      
      Release note (bug fix): prevent spurious "SimpleQuery not allowed while
      in extended protocol mode" errors.
      a6593138
    • craig[bot]'s avatar
      Merge #51079 · a1ee9820
      craig[bot] authored
      51079: kv/gc: don't continue iterating and spamming logs when deadline exceeded r=nvanbenschoten a=nvanbenschoten
      
      Before this change, we'd see logs like:
      ```
      W200706 11:23:10.682177 15143568003 kv/kvserver/gc/gc.go:337  [n118,gc,s118,r6153/1:/Table/57/1/"user1216{19…-44…}] failed to GC a batch of keys: result is ambiguous (context deadline exceeded)
      W200706 11:23:10.685251 15143568003 kv/kvserver/gc/gc.go:337  [n118,gc,s118,r6153/1:/Table/57/1/"user1216{19…-44…}] failed to GC a batch of keys: aborted during Replica.Send: context deadline exceeded
      W200706 11:23:10.688667 15143568003 kv/kvserver/gc/gc.go:337  [n118,gc,s118,r6153/1:/Table/57/1/"user1216{19…-44…}] failed to GC a batch of keys: aborted during Replica.Send: context deadline exceeded
      W200706 11:23:10.691760 15143568003 kv/kvserver/gc/gc.go:337  [n118,gc,s118,r6153/1:/Table/57/1/"user1216{19…-44…}] failed to GC a batch of keys: aborted during Replica.Send: context deadline exceeded
      ...
      ```
      
      if a GC attempt hit its deadline. Now, we terminate iteration early and do our best to stop running GC once the context is canceled / exceeding its deadline.
      Co-authored-by: default avatarNathan VanBenschoten <[email protected]>
      a1ee9820
    • Rebecca Taft's avatar
      opt: fix error in case of casted NULL arguments to AddGeometryColumn · 9743fefe
      Rebecca Taft authored
      This commit fixes an error that occurred when AddGeometryColumn was
      called with NULL arguments that were cast to the type specified by the
      function signature. #50992 already fixed the case when AddGeometryColumn
      was called with bare NULL arguments, since those were detected by TypeCheck.
      TypeCheck does not detect NULL arguments if they are cast to the correct
      type.
      
      This commit fixes the error by adding an explicit check in the optbuilder
      that each argument is not null before calling the SQLFn of the
      AddGeometryColumn overload.
      
      Informs #50296
      
      Release note (bug fix): Fixed an internal error that occurred when
      AddGeometryColumn was called with NULL arguments. This now results in
      a no-op and returns NULL.
      9743fefe
    • craig[bot]'s avatar
      Merge #51062 · 147e3346
      craig[bot] authored
      51062: sql: update logictest blocklist issue number r=asubiotto a=arulajmani
      
      Logictests for sequences were earlier blocked on issue #50840. From the
      discussion on the issue, the root cause is #50048. This PR updates
      the logictest blocklist to reflect that.
      
      Release note: none
      Co-authored-by: default avatararulajmani <[email protected]>
      147e3346
    • Radu Berinde's avatar
      cli: add commands for managing statement diagnostics · 49e6d393
      Radu Berinde authored
      This change adds a `statement-diag` command, with the following subcommands:
      ```
        list        list available bundles and outstanding activation requests
        download    download statement diagnostics bundle into a zip file
        delete      delete statement diagnostics bundles
        cancel      cancel outstanding activation requests
      ```
      
      Fixes #48597.
      
      Release note (cli change): A new set of `statement-diag` CLI commands that can
      be used to manage statement diagnostics.
      49e6d393
    • anzoteh96's avatar
      importccl: fix target column ordering bug for PGDUMP import · 1fc75883
      anzoteh96 authored
      The current implementation assumes that the target columns of a PGDUMP query
      is the same as how they are created in the case where target columns are
      declared in PGDUMP file. This PR addresses it by detecting the target columns
      in the PGDUMP statement itself if this is the case. In addition, given
      that the target columns may not be well-determined at the formation of a
      new `DatumRowConverter`, so the check of unsupported default column
      expression is also moved to the DatumRowConverter.Row() function.
      
      Release note: None
      1fc75883
    • Nathan VanBenschoten's avatar
      kv/gc: don't continue iterating and spamming logs on deadline · 70cce489
      Nathan VanBenschoten authored
      Before this change, we'd see logs like:
      ```
      W200706 11:23:10.682177 15143568003 kv/kvserver/gc/gc.go:337  [n118,gc,s118,r6153/1:/Table/57/1/"user1216{19…-44…}] failed to GC a batch of keys: result is ambiguous (context deadline exceeded)
      W200706 11:23:10.685251 15143568003 kv/kvserver/gc/gc.go:337  [n118,gc,s118,r6153/1:/Table/57/1/"user1216{19…-44…}] failed to GC a batch of keys: aborted during Replica.Send: context deadline exceeded
      W200706 11:23:10.688667 15143568003 kv/kvserver/gc/gc.go:337  [n118,gc,s118,r6153/1:/Table/57/1/"user1216{19…-44…}] failed to GC a batch of keys: aborted during Replica.Send: context deadline exceeded
      W200706 11:23:10.691760 15143568003 kv/kvserver/gc/gc.go:337  [n118,gc,s118,r6153/1:/Table/57/1/"user1216{19…-44…}] failed to GC a batch of keys: aborted during Replica.Send: context deadline exceeded
      ...
      ```
      
      if a GC attempt hit its deadline. Now, we terminate iteration early and
      do our best to stop running GC once the context is canceled / exceeding
      its deadline.
      70cce489
    • craig[bot]'s avatar
      Merge #51167 · 47ef907f
      craig[bot] authored
      51167: geoprojbase: add more NAD83-related SRIDs r=sumeerbhola a=otan
      
      These SRIDs are used in django ORM tests or the PostGIS tutorial.
      
      Also change the format to be consistent between runs, using the full
      string output as well.
      
      Release note: None
      Co-authored-by: default avatarOliver Tan <[email protected]>
      47ef907f
    • Alex Robinson's avatar
      Fix typo in txn.commits metric help text · f4c804a2
      Alex Robinson authored
      f4c804a2
    • craig[bot]'s avatar
      Merge #51025 · 492cde29
      craig[bot] authored
      51025: rpc: use grpc.Chain{Unary,Stream}Interceptor r=nvanbenschoten,andreimatei a=tbg
      
      These were not available when the code was first written but they are
      very helpful for keeping things tidy.
      
      Release note: None
      Co-authored-by: default avatarTobias Schottdorf <[email protected]>
      492cde29
    • Tobias Schottdorf's avatar
      rpc: reverse interceptor order · 9f774508
      Tobias Schottdorf authored
      This addresses the TODO in the previous commit. The salient bit is that
      we want to call the auth interceptor first to avoid exposing the other
      interceptors to unauthed requests.
      
      Release note: None
      9f774508
    • Tobias Schottdorf's avatar
      rpc: use grpc.Chain{Unary,Stream}Interceptor · 46cef8db
      Tobias Schottdorf authored
      These were not available when the code was first written but they are
      very helpful for keeping things tidy.
      
      Follow-up commit will address a TODO introduced here to keep diffs
      reviewable.
      
      Release note: None
      46cef8db
    • Alfonso Subiotto Marques's avatar
      colexec: add CloseAndLogOnErr helper · f023afa9
      Alfonso Subiotto Marques authored
      Release note: None
      f023afa9
    • Alfonso Subiotto Marques's avatar
      colexec: remove mutex from Columnarizer · 66582a73
      Alfonso Subiotto Marques authored
      This mutex was added when DrainMeta and Next could be called concurrently.
      Since this is no longer the case, this commit removes that mutex.
      
      Release note: None (internal code simplification)
      66582a73
    • Alfonso Subiotto Marques's avatar
      colexec: simplify Inbox's DrainMeta method · 88b75fcd
      Alfonso Subiotto Marques authored
      This commit removes unnecessary mutexes and state from the Inbox since
      DrainMeta and Next can no longer be called concurrently.
      
      Release note: None (internal code simplification)
      88b75fcd
    • Alfonso Subiotto Marques's avatar
      colexec: rename IdempotentClose to Close and remove concurrency protections · b5d71e53
      Alfonso Subiotto Marques authored
      Since Close is only ever called from the same goroutine as Next, components
      that would previously need to protect against these concurrent calls no longer
      need to do so.
      
      Release note: None
      b5d71e53
    • Alfonso Subiotto Marques's avatar
      colflow: properly plumb IdempotentClosers in flow · 5540b6ef
      Alfonso Subiotto Marques authored
      Previously IdempotentClosers were not passed on to local outputs and were only
      closed if the direct output of that closer was an outbox or a materializer.
      
      This commit fixes that and adds a logic test testing knob which asserts that
      all components that need to be closed, are closed.
      
      The impact of not doing this was minimal, since flows would clean up any
      temporary files on Cleanup.
      
      Release note: None
      5540b6ef
    • Rafi Shamim's avatar
      roachtest: fix parsing of django test results · efee7c66
      Rafi Shamim authored
      Use a custom test runner class to avoid printing out descriptions of
      test cases, which make it impossible to parse test results in some
      cases.
      
      Also, update the parsing logic to look for "expected failure" and
      "unexpected success" and handle these results appropriately.
      
      Release note: None
      efee7c66
    • craig[bot]'s avatar
      Merge #51161 · 1b5d070c
      craig[bot] authored
      51161: roachtest: refactor tpchvec and introduce tpchvec/bench r=yuzefovich a=yuzefovich
      
      This commit introduces new `tpchvec/bench` config (which is skipped by
      default) that can be used to perform benchmarking of different cluster
      settings against all TPCH queries. This required some refactor of
      existing `tpchvec` variants in order to separate out cluster-wide setup
      queries and more fine-grained pre test run setup.
      
      Release note: None
      Co-authored-by: default avatarYahor Yuzefovich <[email protected]>
      1b5d070c