Use ETag as precondition for partial updates.

Last-Modified does not have enough resolution to be fully reliable;
ETag does. This test now passes on both filesystem and MinIO:

    $ go run ./test/stresspatch -count 100
    ...
    written: 100 of 100

Other S3 implementations haven't been tested.
This commit is contained in:
Catherine
2025-12-04 01:03:57 +00:00
parent 92d6796ad9
commit f5c48d0759
3 changed files with 35 additions and 11 deletions

View File

@@ -217,6 +217,7 @@ func (fs *FSBackend) GetManifest(
}
return manifest, ManifestMetadata{
LastModified: stat.ModTime(),
ETag: fmt.Sprintf("%x", sha256.Sum256(data)),
}, nil
}
@@ -307,6 +308,17 @@ func (fs *FSBackend) checkManifestPrecondition(
}
}
if opts.IfMatch != "" {
data, err := fs.siteRoot.ReadFile(name)
if err != nil {
return fmt.Errorf("read: %w", err)
}
if fmt.Sprintf("%x", sha256.Sum256(data)) != opts.IfMatch {
return fmt.Errorf("%w: If-Match", ErrPreconditionFailed)
}
}
return nil
}

View File

@@ -553,16 +553,17 @@ func (s3 *S3Backend) HasAtomicCAS(ctx context.Context) bool {
func (s3 *S3Backend) checkManifestPrecondition(
ctx context.Context, name string, opts ModifyManifestOptions,
) error {
if !opts.IfUnmodifiedSince.IsZero() {
stat, err := s3.client.StatObject(ctx, s3.bucket, manifestObjectName(name),
minio.GetObjectOptions{})
if err != nil {
return fmt.Errorf("stat: %w", err)
}
stat, err := s3.client.StatObject(ctx, s3.bucket, manifestObjectName(name),
minio.GetObjectOptions{})
if err != nil {
return err
}
if stat.LastModified.Compare(opts.IfUnmodifiedSince) > 0 {
return fmt.Errorf("%w: If-Unmodified-Since", ErrPreconditionFailed)
}
if !opts.IfUnmodifiedSince.IsZero() && stat.LastModified.Compare(opts.IfUnmodifiedSince) > 0 {
return fmt.Errorf("%w: If-Unmodified-Since", ErrPreconditionFailed)
}
if opts.IfMatch != "" && stat.ETag != opts.IfMatch {
return fmt.Errorf("%w: If-Match", ErrPreconditionFailed)
}
return nil
@@ -585,8 +586,16 @@ func (s3 *S3Backend) CommitManifest(
// Remove staged object unconditionally (whether commit succeeded or failed), since
// the upper layer has to retry the complete operation anyway.
putOptions := minio.PutObjectOptions{}
putOptions.Header().Add("X-Tigris-Consistent", "true")
if opts.IfMatch != "" {
// Not guaranteed to do anything (see `HasAtomicCAS`), but let's try anyway;
// this is a "belt and suspenders" approach, together with `checkManifestPrecondition`.
// It does reliably work on MinIO at least.
putOptions.SetMatchETag(opts.IfMatch)
}
_, putErr := s3.client.PutObject(ctx, s3.bucket, manifestObjectName(name),
bytes.NewReader(data), int64(len(data)), minio.PutObjectOptions{})
bytes.NewReader(data), int64(len(data)), putOptions)
removeErr := s3.client.RemoveObject(ctx, s3.bucket, stagedManifestObjectName(data),
minio.RemoveObjectOptions{})
s3.siteCache.Cache.Invalidate(name)

View File

@@ -204,7 +204,10 @@ func PartialUpdateFromArchive(
result = UpdateResult{UpdateError, nil, err}
} else {
result = Update(ctx, webRoot, oldManifest, newManifest,
ModifyManifestOptions{IfUnmodifiedSince: oldMetadata.LastModified})
ModifyManifestOptions{
IfUnmodifiedSince: oldMetadata.LastModified,
IfMatch: oldMetadata.ETag,
})
// The `If-Unmodified-Since` precondition is internally generated here, which means its
// failure shouldn't be surfaced as-is in the HTTP response. If we also accepted options
// from the client, then that precondition failure should surface in the response.