Revert "Merge: [SYSINFRA-554] Deprecate nocacheextern flag (grailbio/…

…grail!3658)" (grailbio/grail!5650)

Approved-by: Jim Clune <jclune@grailbio.com>

GitLab URL: https://gitlab.com/grailbio/grail/-/merge_requests/5650

fbshipit-source-id: cf457c2

blob/mux.go

@@ -124,42 +124,6 @@ func (m Mux) CanTransfer(ctx context.Context, dsturl, srcurl string) (bool, erro
 	return true, nil
 }
 
-// NeedTransfer returns whether src needs to be transferred to the location
-// of dst. It expects both src and dst to be reference files, and it only
-// determines that a transfer is unnecessary if the objects have the same ETag
-// or ContentHash.
-func (m Mux) NeedTransfer(ctx context.Context, dst, src reflow.File) (bool, error) {
-	if src.Size != 0 && dst.Size != 0 && src.Size != dst.Size {
-		return true, nil
-	}
-	// An ETag mismatch doesn't necessarily mean that the objects have different
-	// contents. E.g. the ETag of an S3 object uploaded via multipart copy is
-	// not a digest of the object data (https://docs.aws.amazon.com/AmazonS3/latest/API/API_Object.html).
-	if src.ETag != "" && src.ETag == dst.ETag {
-		return false, nil
-	}
-	// A zero ContentHash doesn't necessarily mean that the field is missing
-	// from the object's metadata.
-	if src.ContentHash.IsZero() {
-		var err error
-		src, err = m.File(ctx, src.Source)
-		if err != nil {
-			return false, err
-		}
-	}
-	if dst.ContentHash.IsZero() {
-		var err error
-		dst, err = m.File(ctx, dst.Source)
-		if err != nil {
-			return false, err
-		}
-	}
-	if !src.ContentHash.IsZero() && !dst.ContentHash.IsZero() {
-		return src.ContentHash != dst.ContentHash, nil
-	}
-	return true, nil
-}
-
 // Transfer transfers the contents of object in srcurl to dsturl.
 // errors.NotSupported is returned if the transfer is not possible.
 func (m Mux) Transfer(ctx context.Context, dsturl, srcurl string) error {

flow/eval.go

@@ -142,10 +142,13 @@ type EvalConfig struct {
 	RunID taskdb.RunID
 
 	// CacheMode determines whether the evaluator reads from
-	// or writes to the cache. If CacheMode is nonzero, Assoc,
+	// or writees to the cache. If CacheMode is nonzero, Assoc,
 	// Repository, and Transferer must be non-nil.
 	CacheMode infra2.CacheMode
 
+	// NoCacheExtern determines whether externs are cached.
+	NoCacheExtern bool
+
 	// RecomputeEmpty determines whether cached empty values
 	// are recomputed.
 	RecomputeEmpty bool
@@ -205,6 +208,11 @@ func (e EvalConfig) String() string {
 	}
 
 	var flags []string
+	if e.NoCacheExtern {
+		flags = append(flags, "nocacheextern")
+	} else {
+		flags = append(flags, "cacheextern")
+	}
 	if e.CacheMode == infra2.CacheOff {
 		flags = append(flags, "nocache")
 	} else {
@@ -838,6 +846,9 @@ func (e *Eval) returnFlow(f *Flow) {
 // incur extra computation, thus dirtying does not work when dirtying
 // nodes are hidden behind maps, continuations, or coercions.
 func (e *Eval) dirty(f *Flow) bool {
+	if !e.NoCacheExtern {
+		return false
+	}
 	if f.Op == Extern {
 		return true
 	}
@@ -884,7 +895,7 @@ func (e *Eval) todo(f *Flow, visited flowOnce, v *FlowVisitor) {
 			}
 		}
 		switch f.Op {
-		case Intern, Exec:
+		case Intern, Exec, Extern:
 			if !e.BottomUp && e.CacheMode.Reading() && !e.dirty(f) {
 				v.Push(f)
 				e.Mutate(f, NeedLookup)
@@ -1113,7 +1124,7 @@ func (e *Eval) CacheWrite(ctx context.Context, f *Flow) error {
 	ctx, endTrace = trace.Start(ctx, trace.Cache, f.Digest(), fmt.Sprintf("cache write %s", f.Digest().Short()))
 	defer endTrace()
 	switch f.Op {
-	case Intern, Exec:
+	case Intern, Extern, Exec:
 	default:
 		return nil
 	}
@@ -1126,6 +1137,12 @@ func (e *Eval) CacheWrite(ctx context.Context, f *Flow) error {
 	if f.Err != nil || f.State != Done {
 		return nil
 	}
+	if f.Op == Data {
+		return nil
+	}
+	if e.NoCacheExtern && f.Op == Extern {
+		return nil
+	}
 	keys := f.CacheKeys()
 	if len(keys) == 0 {
 		return nil
@@ -1235,7 +1252,7 @@ func (e *Eval) batchLookup(ctx context.Context, flows ...*Flow) {
 
 	batch := make(assoc.Batch)
 	for _, f := range flows {
-		if !e.valid(f) || !e.CacheMode.Reading() || f.Op == Extern || f == e.root {
+		if !e.valid(f) || !e.CacheMode.Reading() || e.NoCacheExtern && (f.Op == Extern || f == e.root) {
 			e.lookupFailed(f)
 			continue
 		}