sql: we support bigger range for OID than Postgres #41904
Labels
A-sql-pgcompat
Semantic compatibility with PostgreSQL
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
T-sql-foundations
SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Comments
yuzefovich
added
C-bug
yuzefovich
added this to Triage
in [DEPRECATED] Old SQLExec board. Don't move stuff here
via automation
asubiotto
moved this from Triage
to [TENT] SQL Features
in [DEPRECATED] Old SQLExec board. Don't move stuff here
|
We have marked this issue as stale because it has been inactive for |
|
This is an example of a PR needed because this bit us: #61148 I added TODOs there about making the OID size 4 bytes. We should still do this. |
jlinder
added
the
T-sql-foundations
rafiss
moved this from Shorter term backlog
to Potentially for 22.2
in SQL Sessions - Deprecated
craig bot
pushed a commit
that referenced
this issue
Jun 7, 2022
80792: changefeedccl: update tests to random tenant random sink r=samiskin a=samiskin
Previously most of our tests did not run on tenants as it was not
ergonomic to do so given our helpers. We would also manually run tests
across multiple sinks even if the test did not care about what sink it
was ran on, drastically increasing the execution time of our test suite.
This PR updates our helper infrastructure to use a shared TestServer
struct that allows access to both the system and secondary tenant
interfaces, and by default runs tests on a **random sink** every time.
Running only once per test reduces `pkg/ccl/changefeedccl` test suite
execution time to **under a minute** on a standard employee gceworker.
A couple testing bugs with certain sinks were fixed but some remain
(that simply didn't show up earlier), resulting in some TODOs added on
tests that limit the sinks unnecessarily.
---
The new set of tools look like:
```go
// The main struct that is used by most tests, replaces prior use of the `db *gosql.DB` parameter and `f.Server()`
type TestServer struct {
// DB and Server can either be a System or Secondary tenant, randomly determined or forced through options
DB *gosql.DB
Server serverutils.TestTenantInterface
// SystemDB and SystemServer are always the system tenant, useful for certain tasks that require a full TestServerInterface like getting PublicTableDescriptors or kv stores
SystemDB *gosql.DB
SystemServer serverutils.TestServerInterface
// Some values are accessed differently depending on whether we're using a system or secondary tenant, so that specific logic is done in the makeServer functions
Codec keys.SQLCodec
TestingKnobs base.TestingKnobs
}
testFn := func (t *testing.T, s TestServer, f cdctest.TestFeedFactory) {
... /* very similar content to before */
}
cdcTest(t, testFn, ... /* feedTestOptions like before */)
```
Test options include:
```go
feedTestNoTenants() // Some functionality did not have a straightforward way to make it work for tenants
feedTestForceTenant()
feedTestForceSink("kafka")
// Sinkless is currently by default disallowed from the random selection since many tests
// will assume the existence of a job and I don't want to have someone do their dev while
// never hitting a Sinkless test, CI happens to run on any of the other sinks, then some
// poor non-cdc dev gets hit with a Sinkless failure.
feedTestIncludeSinkless()
// Restrict/omit random selection to/from certain sinks (such as for features that only work on certain sinks)
feedTestRestrictSinks("kafka", "enterprise")
feedTestOmitSinks("cloudstorage")
```
The lower level helpers to make servers and feeds have also changed to make cluster tests easier:
```go
s, serverCleanup := makeTestServer(t);
// Feed factory helper is now detached from server creation to allow easily setting up a feed factory
// on an arbitrary server, such as when manually creating a Cluster.
f, sinkCleanup := makeFeedFactory(t, randomSinkType(), s.Server, s.DB, ... /* feedTestOptions */)
```
Release note: None
82274: backupccl: display up to 10 missing files in `SHOW BACKUP .. with check_files` r=dt a=msbutler
Previously, `SHOW BACKUP WITH check_files` displayed the first missing SST.
This patch will display up to 100 missing SSTs. Further, this renames the
misleading `approximateTablePhysicalSize` to `approximateSpanPhysicalSize`.
Below I write out how physical table size is calculated:
1. Each range we backup maps to 1 to many spans (currently in the
backup_manfest.files object).
2. 1 to many spans get written to an SST. No span will get written to multiple
SSTs.
3. When backup created these spans, it tried really hard to split spans at
table boundaries, so only one table’s data could be in a span, but a last
minute table creation makes this near impossible, due to slow range splits.
A big table will have many spans.
4. To compute the approximate logical size (called size_bytes in SHOW BACKUP)
of each table, we sum the logical bytes over all it’s spans. We identify a
table’s span by checking the table prefix of the first key in the span. See
getTableSizes method)
5. To compute the physical size (file_bytes in SHOW BACKUP) of a span, compute
the logical size of each SST by summing the logical bytes in the SST over its
spans (see getLogicalSSTSize method), and attribute a portion of the physical
SST size (returned from cloud storage) to a span using the formula:
(sstPhysicalSize) * (logicalSpanSize) / (logicalSSTSize) = physicalSpanSize (
the approximateSpanTableSize method implements this).
6. To compute the physical size of a table, sum over the physical sizes the
table’s spans
Release note (sql change): SHOW BACKUP WITH check_files will display up to 10
missing SSTs.
82329: kv, gossip: remove misc deprecated system config code r=RichardJCai a=RichardJCai
Release note: None
82418: dev: various improvements to `dev generate cgo` and friends r=mari-crl a=rickystewart
1. Up until this point, `dev generate go` has not included the
`zcgo_flags.go` sources, which has been a point of confusion for
people who expect `dev generate go` to generate all the .go files.
Now `dev generate go` includes `cgo` as well, and there is a new
target `dev generate go_nocgo` that does what `dev generate go` used
to do.
2. Now `dev generate cgo` is conscious of where `force_build_cdeps` is
set. If it is, then we make sure not to check in one of the
pre-archived locations. To this end we add a `test_force_build_cdeps`
target that `dev generate cgo` builds.
Release note: None
82430: sql: use uint32 for DOid r=otan a=rafiss
refs #41904
This resolves a piece of tech debt that has caused us a few surprises
and bugs in the past. This doesn't change anything about the on-disk
representation -- it just makes it so that OIDs handled in memory are
more reliably unsigned 32 bit integers.
Release note: None
82443: storage: tweak `newMVCCIterator()` r=jbowens a=erikgrinaker
This patch tweaks `newMVCCIterator()` for use with MVCC range
tombstones, and uses it for all appropriate MVCC operations.
Release note: None
82530: cloud: bump orchestrator to v22.1.1 r=e-mbrown a=e-mbrown
Release note: None
Co-authored-by: Shiranka Miskin <shiranka.miskin@gmail.com>
Co-authored-by: Michael Butler <butler@cockroachlabs.com>
Co-authored-by: richardjcai <caioftherichard@gmail.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
Co-authored-by: e-mbrown <ebsonari@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Here is a simple repro:
As mentioned by Matt and Raphael, this can "bite us" in the future. The concerns are:
Epic CRDB-14489
Jira issue: CRDB-5408
The text was updated successfully, but these errors were encountered: