kv: use origin timestamp to validate lww semantics

[jeffswenson]

The PR updates the implementation of put, cput, delete, and range delete
to return an error if a replicated write with an origin timestamp would
overwrite a value with a more recent origin or MVCC timestamp.

No explicit version checks are included. In a mixed-version environment,
a SQL writer running 25.1 and writing to a 25.2 KV server will encounter
"condition failed" errors if an insert would overwrite a tombstone and
retry the write until it is upgraded or the LDR PTS ages out. This is
acceptable, as LDR remains in preview and the SQL writer is neither the
default nor the recommended writer.

Release note: none
Epic: CRDB-48647

[cockroach-teamcity]

This change is 

[jeffswenson]

This change depends on #143096. It was split out from #142368.

[stevendanna]

Overall this seems reasonable to me.

I left some nits but take them or leave them.

[stevendanna]

If you wanted, you could construct a get with the option to return full mvcc value headers to verify that the value header was set. But I think the mvcc history tests probably cover that well enough so I leave it to your judgement.

[jeffswenson]

Is that actually possible? I considered this when writing the test, but cmd_get.go has no way to set Tombstone: true on the MVCCGetOptions.

cockroach/pkg/kv/kvserver/batcheval/cmd_get.go

Line 40 in 8c96475

getRes, err := storage.MVCCGet(ctx, readWriter, args.Key, h.Timestamp, storage.MVCCGetOptions{

And I decided it wasn't worth the trouble to figure out how to use scan for this since the LDR tests and the mvcc tests verify the actual origin timestamp.

[jeffswenson]

Thanks for the review!

bors r+

[craig]

Build failed (retrying...):

• local_roachtest

[craig]

Build succeeded:

• linux_amd64_fips_build
• local_roachtest
• lint
• linux_amd64_build
• local_roachtest_fips
• docker_image_amd64
• check_generated_code
• examples_orms
• acceptance
• unit_tests

[michae2]

Random thought: when the time comes to support FK checks and unique checks during LDR SQL writes, I wonder if the Get / Scan performing the check will need to use the OriginTimestamp to read "underneath" a newer value, in order to avoid a spurious FK violation when the foreign row has been modified more recently than the OriginTimestamp write?

[jeffswenson]

Origin timestamp is only validated when writing KVs. So I would expect the foreign key checks to work since they are reads.

[michae2]

Is there something in LDR that keeps the most-recently-written OriginTimestamp consistent across rows?

What I'm imagining is a modification history on the origin cluster like:

CREATE TABLE parent (p INT PRIMARY KEY);
CREATE TABLE child (c INT PRIMARY KEY, p INT REFERENCES parent (p));
INSERT INTO parent VALUES (1);
INSERT INTO child VALUES (1, 1);
DELETE FROM child WHERE c = 1;
DELETE FROM parent WHERE p = 1;

Is there something in LDR that ensures the delete from parent always occurs after the insert and delete of child? If not, then it seems like when we replay the insert into child, and perform a FK check, we'll need that FK check to happen at the correct MVCC time, which we can find out from the OriginTimestamp that all events are at.

[jeffswenson]

The SQL layer must ensure that, at write time, foreign key and uniqueness checks pass based on the local cluster’s state. It's acceptable for a row to fail these checks even if it would have passed on the source cluster at that moment.

Currently, the system retries writes that fail foreign key or uniqueness checks, allowing it to eventually apply writes that arrive out of order. If a row continues to fail after several retries, it is moved to the DLQ.

This behavior is one reason I believe foreign key and uniqueness constraints may be ill-suited for tables replicated via LDR. These constraints don't guarantee convergence and are likely to result in rows being sent to the DLQ. In the future, we may be able to improve this by enforcing a deterministic order for replicated writes, but ensuring that order would fall under LDR’s responsibility, not the SQL layer’s.

[jeffswenson]

One quirk in the current implementation is that we set an origin timestamp on all index writes, even though only the timestamp on the primary key is actually load bearing. For instance, if a row is inserted and then updated without modifying the indexed fields, the indexes retain the original insert timestamp rather than reflecting the update.

[michae2]

Oh, hmm, maybe deferred checks make this easier.