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

dev,bazel: miscellaneous paper cuts #75453

Closed
20 of 28 tasks
irfansharif opened this issue Jan 24, 2022 · 1 comment
Closed
20 of 28 tasks

dev,bazel: miscellaneous paper cuts #75453

irfansharif opened this issue Jan 24, 2022 · 1 comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-quick-win Likely to be a quick win for someone experienced. T-dev-inf

Comments

@irfansharif
Copy link
Contributor

irfansharif commented Jan 24, 2022

Describe the problem

Here's an assorted list of paper cuts we should address. They're grouped by size and should ease the transition away from make as a daily driver. Collecting them here for the Bazel CS team + volunteers.

Small:

  • --rewrite should probably imply --ignore-cache
  • list all supported build targets as part of 'dev build -h'
  • stop munging ccache paths through dev to reduce build thrashing when switching between bazel/dev
  • introduce a top-level --test-arg to pass arguments directly to the go test binary (see 'go help testflags' for all you can do); get rid of --rewrite-arg while here.
  • when running test/benchmark with '-{cpu,mem}profile' passed to the underlying go test binary, it's not clear where the pprof output is placed
  • support --stress{,-args}, --count for 'dev testlogic' (maybe unify the code paths with 'dev test'?)
  • support --show-sql for 'dev testlogic'
  • disable test sharding (--test_sharding_strategy=disabled) when --filter is specified; for sharded (like pkg/kv/kvserver) packages when filtering for a specific test, there's a lot of additional test runner output for spawns that don't do anything.
  • symlink 'cockroach' to the 'cockroach-short' binary for 'dev build short'
  • maybe stream 'dev test' output by default (i.e. use --test_output streamed instead), or provide a flag for it serializes test runs which may be undesirable
  • bazel,dev: cannot rewrite from different package #76513
  • dev test pkg/kv/kvserver -f TestNonExistent succeeds without any warnings; we could be peeking at the generated test.xml and spit out something:
$ cat _bazel/testlogs/pkg/kv/kvserver/kvserver_test/test.xml
dev bench pkg/sql/colexec/colexecjoin -f=MergeJoiner/rows=32768 --count=5 --cpus=1 --test-args="-test.cpuprofile=cpu.out" --bench-mem
go tool pprof -http localhost:8081 cpu.out # navigate to "Source" view and see that listings for .eg.go files aren't resolvable
  • pass '-no_warning_for_no_symbols' to libtool/ar on macs (gets rid of benign "has no symbols" warnings, hint)
  • top-level dev clean (aliasing bazel clean {--expunge} and re-parenting dev ui clean --all to come underneath it
  • dev gen bazel --short to only run gazelle

Medium:

Epic CRDB-17171

Jira issue: CRDB-12679

@irfansharif irfansharif added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-dev-inf E-quick-win Likely to be a quick win for someone experienced. labels Jan 24, 2022
craig bot pushed a commit that referenced this issue Feb 12, 2022
76394: sql: use IndexFetchSpec in TableReader r=RaduBerinde a=RaduBerinde

#### row: replace fetcher args with IndexFetchSpec

This commit replaces the row fetcher table args with IndexFetchSpec.

Release note: None

#### sql: use IndexFetchSpec in TableReader

This commit removes the table descriptor from TableReader and replaces
it with an IndexFetchSpec.

Eventually, we will use the same `IndexFetchSpec` to form the columnar
batch directly in KV.

Release note: None


76415: dev: address a few paper cuts r=irfansharif a=irfansharif

See individual commits, works through a few of the known paper cuts with dev/bazel (#75453).

76420: sql: initialize tenant span config with rangefeeds enabled r=irfansharif a=ajwerner

Not doing this causes problems when we construct new tenants as they won't be
able to establish a rangefeed until their first full reconciliation completes
and then propagates.

Even this is not great. If the preceding range did not have rangefeeds enabled,
it would take a closed timestamp interval for this enablement to propagate.
Perhaps this is evidence that we should always carve out a span at the end of
the keyspace and set it to have rangefeeds enabled.

I'll leave that fix and testing of this to somebody else. Hope this is helpful
enough on its own. cc `@irfansharif`  and `@arulajmani.` I believe this problem
has been blocking `@RaduBerinde.`

Fixes #76331

Release note: None

76437: clisqlshell: handle interactive query cancellation r=rafiss a=knz

Fixes #76433. 

As of this PR, there's a bug in lib/pq which forces the session to
terminate when any query gets cancelled. We find this unacceptable
and we plan to fix it later.

Release note (cli change): The interactive SQL shell (`cockroach sql`,
`cockroach demo`) now supports interrupting a currently running
query with Ctrl+C, without losing access to the shell.

76442: roachtest: try to stabilize ORM nightlies r=RichardJCai a=rafiss

fixes #76428
fixes #76426
fixes #68363
touches #76231
touches #76016
touches #76011

Ruby-pg and psycopg2 tests started passing because we now support pgwire
cancel.

Other tests (Django, Hibernate, and PGJDBC) seem flaky after the testing
settings change, so try to use slightly more conservative values.

Release note: None

76450: ci: add extra logging of `$TC_SERVER_URL` in pebble metamorphic nightly r=nicktrav a=rickystewart

Release note: None

Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Andrew Werner <awerner32@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
@cockroachdb cockroachdb deleted a comment from mari-crl May 26, 2022
@rickystewart
Copy link
Collaborator

I'm closing this. Most of the remaining paper cuts either already have GitHub issues set up, or they're tasks which I'm uncertain we'll actually implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-quick-win Likely to be a quick win for someone experienced. T-dev-inf
Projects
None yet
Development

No branches or pull requests

2 participants