importccl,backupccl: import is erroneously relying on string compares to detect particular errors #35942
Labels
A-disaster-recovery
C-cleanup
Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.
T-disaster-recovery
Comments
knz
added
C-cleanup
knz
added a commit
to knz/cockroach
that referenced
this issue
Mar 19, 2019
This patch ensures that the full error message (pre-unwrap) is included for errors that flow out of distsql process towards the gateway. This is important because some other internal mechanisms inside CockroachDB are expecting to see the prefixes introduced by errors.Wrap to decide other things (a bad idea, but not fixable in the short term), for example see issue cockroachdb#35942. Release note: None
knz
added a commit
to knz/cockroach
that referenced
this issue
Mar 19, 2019
This patch ensures that the full error message (pre-unwrap) is included for errors that flow out of distsql process towards the gateway. This is important because some other internal mechanisms inside CockroachDB are expecting to see the prefixes introduced by errors.Wrap to decide other things (a bad idea, but not fixable in the short term), for example see issue cockroachdb#35942. Release note: None
|
So fortunately that code path can go away soon as we no longer need to handle that once we're on cluster version 19.1+ I thought that |
|
Doesn't matter where it comes from if it gets stripped by errors.Cause. I'm glad the problem will go away though I'm left to ask: how?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
Huh, really? where does that happen? |
craig bot
pushed a commit
that referenced
this issue
Mar 19, 2019
35912: roachtest: begin the release qualification roachtest whitelist r=nvanbenschoten a=danhhz Starting with `tpcc/nodes=3/w=max` for now so I can get the teamcity plumbing done. This test verifies that running our claimed max TPCC warehouses works. The max depends on the hardware configuration: currently 1450 on a 3-node GCE n1-standard-1 cluster or 2200 on a 3-node AWS c5d.4xlarge cluster. Release note: None 35915: sql: do not create stats on inverted index columns r=rytaft a=rytaft This commit fixes an issue in which the default columns for `CREATE STATISTICS` included inverted index columns. Since we cannot create statistics on JSON columns, running automatic statistics on a table with an inverted index resulted in the error "unable to encode table key: *tree.DJSON". This commit fixes the issue by skipping over inverted indexes when determining the default columns for `CREATE STATISTICS`. Fixes #35764 Release note (bug fix): Fixed an error that occurred when creating statistics on tables with an inverted index. 35944: distsqlpb,importccl: fix a regression in IMPORT r=RaduBerinde a=knz Fixes #35943. This patch ensures that the full error message (pre-unwrap) is included for errors that flow out of distsql process towards the gateway. This is important because some other internal mechanisms inside CockroachDB are expecting to see the prefixes introduced by errors.Wrap to decide other things (a bad idea, but not fixable in the short term), for example see issue #35942. Release note: None Co-authored-by: Daniel Harrison <[email protected]> Co-authored-by: Rebecca Taft <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
|
oh, sorry, I was thinking of a different |
It certainly can using the technique in #35920 or something like it. |
|
Closing for now, since it’s RocksDB. We think there might be improvements, but will track elsewhere. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The code for RESTORE (and thus IMPORT CSV) is using rocksdb ingest sst's built-in duplicate detection to report whether the ingested data is valid for a primary or unique index.
Any rocksdb error seems to be compared by a test on the text of the error message, it has to be prefixed specifically by
Invalid argument: Keys ....The problem is that the
Invalid argument:part is added byerrors.Wrap(), so that if the error gets unwrapped it does not match any more.The check for the error should be made more robust, for example using the mechanism introduced in #35920.
The text was updated successfully, but these errors were encountered: