Merge cockroachdb#107174 cockroachdb#107188

107174: testserver: avoid nodelocal capability in some cases r=knz a=stevendanna

Waiting for the nodelocal capability takes time because we won't see the update until the closed timestamp for the relevant ranges advance.

We could, and perhaps should, lower the closed timestamp target in tests. Or perhaps make other changes that avoid this long wait.

But, while we decide on what to do, we can avoid doing this at all for tests that can't possibly be using nodelocal storage because they haven't set an external storage dir.

Epic: CRDB-18499

Release note: None

107188: backup/kvserver: plumb backing file size through addSSTablePreApply()  r=dt a=msbutler

Epic: none

Release note: none

Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Michael Butler <[email protected]>

Author: 3 people

pkg/ccl/backupccl/restore_job.go

@@ -3202,7 +3202,11 @@ func sendAddRemoteSSTs(
 				}
 			}
 
-			loc := kvpb.AddSSTableRequest_RemoteFile{Locator: uris[i], Path: file.Path}
+			loc := kvpb.AddSSTableRequest_RemoteFile{
+				Locator:         uris[i],
+				Path:            file.Path,
+				BackingFileSize: file.BackingFileSize,
+			}
 			// TODO(dt): see if KV has any better ideas for making these up.
 			fileStats := &enginepb.MVCCStats{
 				ContainsEstimates: 1,

pkg/ccl/backupccl/restore_span_covering.go

@@ -476,7 +476,9 @@ func generateAndSendImportSpans(
 					Path:                  f.Path,
 					Dir:                   backups[layer].Dir,
 					BackupFileEntrySpan:   f.Span,
-					BackupFileEntryCounts: f.EntryCounts}
+					BackupFileEntryCounts: f.EntryCounts,
+					BackingFileSize:       f.BackingFileSize,
+				}
 				if dir, ok := backupLocalityMap[layer][f.LocalityKV]; ok {
 					fileSpec.Dir = dir
 				}

pkg/kv/kvpb/api.proto

@@ -2004,9 +2004,11 @@ message AddSSTableRequest {
   //
   // TODO(dt, msbutler, bilal): This is unsupported.
   // TOOD(dt, msbutler, bilal): support sst_timestamp_to_request_timestamp.
+  // TODO(msbutler): rename to ExternalFile.
   message  RemoteFile {
     string locator = 1;
     string path = 2;
+    uint64 backing_file_size = 3;
   }
   RemoteFile remote_file = 10 [(gogoproto.nullable) = false];
 

pkg/kv/kvserver/batcheval/cmd_add_sstable.go

@@ -174,9 +174,10 @@ func EvalAddSSTable(
 		return result.Result{
 			Replicated: kvserverpb.ReplicatedEvalResult{
 				AddSSTable: &kvserverpb.ReplicatedEvalResult_AddSSTable{
-					RemoteFileLoc:  args.RemoteFile.Locator,
-					RemoteFilePath: args.RemoteFile.Path,
-					Span:           roachpb.Span{Key: start.Key, EndKey: end.Key},
+					RemoteFileLoc:   args.RemoteFile.Locator,
+					RemoteFilePath:  args.RemoteFile.Path,
+					BackingFileSize: args.RemoteFile.BackingFileSize,
+					Span:            roachpb.Span{Key: start.Key, EndKey: end.Key},
 				},
 				MVCCHistoryMutation: mvccHistoryMutation,
 			},

pkg/kv/kvserver/kvserverpb/proposer_kv.proto

@@ -193,9 +193,11 @@ message ReplicatedEvalResult {
     // taken place since 22.1, to make sure all Raft log entries from 21.2 or
     // older have been applied on all replicas.
     bool at_write_timestamp = 4;
-    
+
+    // TODO(msbutler,dt): bikeshed the name of these fields.
     string remote_file_loc = 5;
     string remote_file_path = 6;
+    uint64 backing_file_size = 7;
   }
   AddSSTable add_sstable = 17 [(gogoproto.customname) = "AddSSTable"];
 

pkg/kv/kvserver/replica_proposal.go

@@ -601,7 +601,9 @@ func addSSTablePreApply(
 ) bool {
 	if sst.RemoteFilePath != "" {
 		// TODO(dt, bilal, msbutler): Replace this with eng.IngestRemoteFile()
-		log.Warningf(ctx, "EXPERIMENTAL AddSSTABLE REMOTE FILE UNSUPPORTED; downloading %s from %s and adding it whole, ignoring span %s", sst.RemoteFilePath, sst.RemoteFileLoc, sst.Span)
+		log.Warningf(ctx, "EXPERIMENTAL AddSSTABLE REMOTE FILE UNSUPPORTED; downloading %s ("+
+			"with size %d) from %s and adding it whole, ignoring span %s", sst.RemoteFilePath,
+			sst.BackingFileSize, sst.RemoteFileLoc, sst.Span)
 		s, err := env.external.OpenURL(ctx, sst.RemoteFileLoc, username.SQLUsername{})
 		if err != nil {
 			log.Fatalf(ctx, "failed to open remote location %q below raft: %v", sst.RemoteFileLoc, err)

pkg/server/testserver.go

@@ -1146,7 +1146,21 @@ func (ts *TestServer) StartTenant(
 	baseCfg.DisableTLSForHTTP = params.DisableTLSForHTTP
 	baseCfg.EnableDemoLoginEndpoint = params.EnableDemoLoginEndpoint
 
-	if ts.ClusterSettings().Version.IsActive(ctx, clusterversion.V23_1TenantCapabilities) {
+	// Waiting for capabilities can take 3+ seconds since the
+	// rangefeedcache needs to wait for the closed timestamp
+	// before flushing updates. To avoid paying this cost in all
+	// cases, we only set the nodelocal storage capability if the
+	// caller has configured an ExternalIODir since nodelocal
+	// storage only works with that configured.
+	//
+	// TODO(ssd): We should do more here. We could have the caller
+	// pass in explicitly that they want these capabilities. Or,
+	// we could modify the system in some way to avoid waiting on
+	// capabilities for so long. Also, note that this doesn't
+	// apply to StartSharedProcessTenant.
+	shouldGrantNodelocalCap := ts.params.ExternalIODir != ""
+	canGrantNodelocalCap := ts.ClusterSettings().Version.IsActive(ctx, clusterversion.V23_1TenantCapabilities)
+	if canGrantNodelocalCap && shouldGrantNodelocalCap {
 		_, err := ie.Exec(ctx, "testserver-alter-tenant-cap", nil,
 			"ALTER TENANT [$1] GRANT CAPABILITY can_use_nodelocal_storage", params.TenantID.ToUint64())
 		if err != nil {

pkg/sql/execinfrapb/processors_bulk_io.proto

@@ -294,6 +294,7 @@ message RestoreFileSpec {
   reserved 4;
   optional roachpb.Span backup_file_entry_span = 5 [(gogoproto.nullable) = false];
   optional roachpb.RowCount backup_file_entry_counts = 6 [(gogoproto.nullable) = false];
+  optional uint64 backing_file_size = 7 [(gogoproto.nullable) = false];
 }
 
 message TableRekey {