Skip to content

Commit 359e775

Browse files
committed
row: remove InitChecksum for each KV inserted by IMPORT
I noticed `InitChecksum` call show up in the CPU profile of a node from 300 node cluster when IMPORT is running, and I was confused by it. I did some archaeology, and I think it hasn't been needed for a long time. For context, prior to 2.0, we effectively required setting the checksum on the Value since evaluation of CPuts compared RawBytes directly, which included the checksum comparison. This was an oversight because the checksum should be optional, and it became such in 33957ae. However, in order to support IMPORT in mixed-version 1.1-2.0 cluster, in 5ae1bb2 we added the population of the checksum for all KVs produced for insertion. There is a discussion on the PR that this is no longer needed as of the optional checksum change mentioned above. There is also a very recent discussion about removing this checksum in non-test code altogether since it probably doesn't give us much since we do checksum verification at the storage / network level. See #145541. It should be safe to not populate the checksum for IMPORT as it seems to be the only place we do so in the SQL land. Release note: None
1 parent 98ab43e commit 359e775

File tree

2 files changed

+3
-4
lines changed

2 files changed

+3
-4
lines changed

pkg/sql/importer/import_stmt_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5330,9 +5330,9 @@ func TestImportWorkerFailure(t *testing.T) {
53305330
)
53315331
}
53325332

5333-
// TestImportMVCCChecksums verifies that MVCC checksums are correctly
5334-
// computed by issuing a secondary index change that runs a CPut on the
5335-
// index. See #23984.
5333+
// TestImportMVCCChecksums is a regression test for #23984 where we didn't set
5334+
// the MVCC checksums on roachpb.Values produced by the IMPORT when it was
5335+
// required. Doing so is now optional.
53365336
func TestImportMVCCChecksums(t *testing.T) {
53375337
defer leaktest.AfterTest(t)()
53385338
defer log.Scope(t).Close(t)

pkg/sql/row/row_converter.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,6 @@ func NewDatumRowConverter(
349349
db: db,
350350
}
351351
c.kvInserter = func(kv roachpb.KeyValue) {
352-
kv.Value.InitChecksum(kv.Key)
353352
c.KvBatch.KVs = append(c.KvBatch.KVs, kv)
354353
c.KvBatch.MemSize += int64(cap(kv.Key) + cap(kv.Value.RawBytes))
355354
}

0 commit comments

Comments
 (0)