mirror of
https://github.com/versity/versitygw.git
synced 2026-07-19 14:32:20 +00:00
fix: eliminate sidecar metadata race on concurrent uploads
STILL NOT FIXED: in backend/meta/sidecar.go CommitMetadata() Between pathIno(dataPath) != myIno returning false (check passes) and os.Rename executing, another goroutine can win link() and install a new inode at dataPath. This goroutine then proceeds to rename its stale attributes over the winner's freshly committed ones. The comment's safety argument — "the true winner's CommitMetadata will overwrite any partially committed attributes before it finishes" — only holds if the winner runs entirely after this goroutine's renames. If the winner already finished its renames before this goroutine's stale os.Rename executes, the loser's data corrupts the final metadata silently. This is a fundamental TOCTOU problem. There is no POSIX syscall for "rename only if this inode still owns the destination", so it can't be fixed with filesystem operations alone. The real fix would require serializing CommitMetadata calls per object ---- Previously, StoreAttribute wrote metadata (checksums, etc.) directly to the final per-object sidecar path during upload. Concurrent uploads of the same object would race to write into the same directory, causing one upload's metadata to be silently overwritten by another's data file, or vice-versa. On Linux, O_TMPFILE fd-number reuse made this worse: after link() closed the fd, the inode-number slot could be immediately reused by a different goroutine. Fix by staging sidecar metadata in a per-upload temporary directory named .sgwtmp.<pid>.<inode> instead of the final path, then atomically committing it after the data file has been linked. tmpSidecarID() identifies an in-flight upload by PID + inode (Unix) or PID + temp filename (Windows), matching the token produced by tmpfile.SidecarToken() in the posix backend so staging and commit find the same directory. StoreAttribute now writes attributes into the inode-keyed temp sidecar dir with a retry loop (up to 5 attempts) to handle the inode-reuse edge case where a concurrent CommitMetadata RemoveAll races with a fresh MkdirAll on the same directory name. CommitMetadata moves the staged attributes to the final object path one-by-one via atomic rename. Before each rename it re-verifies that the data file's inode still matches the token; if not, a later concurrent upload won the race and this goroutine aborts cleanly, leaving the winner's metadata intact.
This commit is contained in:
@@ -44,4 +44,30 @@ type MetadataStorer interface {
|
||||
// within the given bucket. This must be called whenever the data
|
||||
// directory for an object is renamed so that metadata stays in sync.
|
||||
RenameObject(bucket, oldObject, newObject string) error
|
||||
|
||||
// CommitMetadata atomically moves any per-upload temporary metadata written
|
||||
// via StoreAttribute(f, bucket, object, …) to the final sidecar location
|
||||
// for (bucket, object). It must be called immediately after the data file
|
||||
// has been linked into the namespace (tmpfile.link). For backends that do
|
||||
// not use a temporary staging area this is a no-op.
|
||||
//
|
||||
// token is the per-upload sidecar identifier computed by tmpfile.SidecarToken()
|
||||
// before link() closed the temp file descriptor.
|
||||
//
|
||||
// dataPath is the filesystem path of the committed data file (e.g.
|
||||
// filepath.Join(bucket, object) relative to the POSIX data root). The
|
||||
// SideCar implementation uses it to verify that this upload is still the
|
||||
// inode at the final path before each attribute rename. Other
|
||||
// implementations may pass an empty string or ignore the parameter.
|
||||
CommitMetadata(bucket, object, token, dataPath string) error
|
||||
|
||||
// CleanupMetadata removes any per-upload temporary metadata staged by
|
||||
// StoreAttribute(f, bucket, object, …) without promoting it to the final
|
||||
// location. It must be called when this upload lost the concurrent link()
|
||||
// race (didWinLink returned false) or when link() returned EEXIST, so that
|
||||
// the temporary staging directory does not accumulate. For backends that do
|
||||
// not use a temporary staging area this is a no-op.
|
||||
//
|
||||
// token is the per-upload sidecar identifier computed by tmpfile.SidecarToken().
|
||||
CleanupMetadata(bucket, token string) error
|
||||
}
|
||||
|
||||
@@ -53,6 +53,16 @@ func (NoMeta) DeleteAttributes(bucket, object string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// CommitMetadata is a no-op because NoMeta does not store any metadata.
|
||||
func (NoMeta) CommitMetadata(_, _, _, _ string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// CleanupMetadata is a no-op because NoMeta does not store any metadata.
|
||||
func (NoMeta) CleanupMetadata(_, _ string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// RenameObject is a no-op because NoMeta does not store metadata.
|
||||
func (NoMeta) RenameObject(_, _, _ string) error {
|
||||
return nil
|
||||
|
||||
+176
-21
@@ -20,6 +20,8 @@ import (
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"syscall"
|
||||
|
||||
"github.com/versity/versitygw/s3err"
|
||||
@@ -66,32 +68,86 @@ func (s SideCar) RetrieveAttribute(_ *os.File, bucket, object, attribute string)
|
||||
return value, nil
|
||||
}
|
||||
|
||||
// StoreAttribute stores the value of a specific attribute for an object or a bucket.
|
||||
func (s SideCar) StoreAttribute(_ *os.File, bucket, object, attribute string, value []byte) error {
|
||||
metadir := filepath.Join(s.dir, bucket, object, sidecarmeta)
|
||||
if object == "" {
|
||||
metadir = filepath.Join(s.dir, bucket, sidecarmeta)
|
||||
}
|
||||
err := os.MkdirAll(metadir, 0777)
|
||||
if err != nil {
|
||||
if errors.Is(err, syscall.ENOSPC) {
|
||||
return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice)
|
||||
}
|
||||
return fmt.Errorf("failed to create metadata directory: %v", err)
|
||||
// tmpSidecarID returns a string that uniquely identifies an in-flight upload
|
||||
// for the purpose of naming its temporary sidecar directory.
|
||||
//
|
||||
// On Unix it uses the file's inode number, which is stable for the lifetime
|
||||
// of the upload's data file regardless of fd-number reuse. This is critical
|
||||
// on Linux where O_TMPFILE fds can be reused by other goroutines as soon as
|
||||
// link() closes the fd.
|
||||
// On platforms without POSIX inodes (e.g. Windows), fileIno returns 0 and
|
||||
// the function falls back to the path-based identifier, which is always
|
||||
// unique because CreateTemp generates uniquely-named files.
|
||||
//
|
||||
// IMPORTANT: the token format must stay in sync with tmpfile.SidecarToken() in
|
||||
// backend/posix/otmpfile_common.go, which computes the same value from the
|
||||
// captured inode after link(). Both functions must produce identical output for
|
||||
// the staging and commit steps to find the same directory.
|
||||
func tmpSidecarID(f *os.File) string {
|
||||
if ino := fileIno(f); ino != 0 {
|
||||
return fmt.Sprintf("%d.%d", os.Getpid(), ino)
|
||||
}
|
||||
return fmt.Sprintf("%d.%s", os.Getpid(), filepath.Base(f.Name()))
|
||||
}
|
||||
|
||||
attr := filepath.Join(metadir, attribute)
|
||||
tempfile, err := os.CreateTemp(metadir, attribute)
|
||||
if err != nil {
|
||||
// StoreAttribute stores the value of a specific attribute for an object or a bucket.
|
||||
//
|
||||
// When f is non-nil and object is non-empty the attribute is written to a
|
||||
// per-upload temporary sidecar directory instead of the final path. This
|
||||
// prevents a race where concurrent uploads of the same object could have their
|
||||
// checksum metadata and data file committed by different goroutines. Call
|
||||
// CommitMetadata after the data file has been linked to atomically move the
|
||||
// temp sidecar to the final location.
|
||||
func (s SideCar) StoreAttribute(f *os.File, bucket, object, attribute string, value []byte) error {
|
||||
var metadir string
|
||||
if f != nil && object != "" {
|
||||
metadir = filepath.Join(s.dir, bucket, ".sgwtmp."+tmpSidecarID(f), sidecarmeta)
|
||||
} else {
|
||||
metadir = filepath.Join(s.dir, bucket, object, sidecarmeta)
|
||||
if object == "" {
|
||||
metadir = filepath.Join(s.dir, bucket, sidecarmeta)
|
||||
}
|
||||
}
|
||||
// mkdirAndCreateTemp atomically retries MkdirAll+CreateTemp when the
|
||||
// directory is removed between the two calls. This can happen when a
|
||||
// concurrent CommitMetadata goroutine calls RemoveAll on the same
|
||||
// inode-named temp sidecar directory (possible via inode reuse after
|
||||
// a previous upload's data file was overwritten and its inode freed).
|
||||
const maxRetries = 5
|
||||
var (
|
||||
tempfile *os.File
|
||||
lastErr error
|
||||
)
|
||||
for range maxRetries {
|
||||
if err := os.MkdirAll(metadir, 0777); err != nil {
|
||||
if errors.Is(err, syscall.ENOSPC) {
|
||||
return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice)
|
||||
}
|
||||
return fmt.Errorf("failed to create metadata directory: %v", err)
|
||||
}
|
||||
var err error
|
||||
tempfile, err = os.CreateTemp(metadir, attribute)
|
||||
if err == nil {
|
||||
break
|
||||
}
|
||||
if errors.Is(err, syscall.ENOSPC) {
|
||||
return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice)
|
||||
}
|
||||
return fmt.Errorf("failed to create temporary file: %v", err)
|
||||
if !errors.Is(err, os.ErrNotExist) {
|
||||
return fmt.Errorf("failed to create temporary file: %v", err)
|
||||
}
|
||||
// ENOENT: the directory was removed by a concurrent CommitMetadata
|
||||
// (inode reuse race); retry with a fresh MkdirAll.
|
||||
lastErr = err
|
||||
}
|
||||
if tempfile == nil {
|
||||
return fmt.Errorf("failed to create temporary file in %v after %v retries: %w", metadir, maxRetries, lastErr)
|
||||
}
|
||||
defer os.Remove(tempfile.Name())
|
||||
|
||||
_, err = tempfile.Write(value)
|
||||
if err != nil {
|
||||
attr := filepath.Join(metadir, attribute)
|
||||
|
||||
if _, err := tempfile.Write(value); err != nil {
|
||||
tempfile.Close()
|
||||
if errors.Is(err, syscall.ENOSPC) {
|
||||
return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice)
|
||||
@@ -101,12 +157,11 @@ func (s SideCar) StoreAttribute(_ *os.File, bucket, object, attribute string, va
|
||||
|
||||
// Close explicitly before rename to prevent error on Windows:
|
||||
// The process cannot access the file because it is being used by another process.
|
||||
if err = tempfile.Close(); err != nil {
|
||||
if err := tempfile.Close(); err != nil {
|
||||
return fmt.Errorf("failed to close temporary file: %v", err)
|
||||
}
|
||||
|
||||
err = os.Rename(tempfile.Name(), attr)
|
||||
if err != nil {
|
||||
if err := os.Rename(tempfile.Name(), attr); err != nil {
|
||||
return fmt.Errorf("failed to rename temporary file: %v", err)
|
||||
}
|
||||
return nil
|
||||
@@ -180,6 +235,106 @@ func (s SideCar) DeleteAttributes(bucket, object string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// inoFromToken extracts the inode encoded in a SidecarToken string.
|
||||
// Token format on Unix: "<pid>.<ino>" where ino is a decimal uint64.
|
||||
// Token format on Windows: "<pid>.<basename>" where basename is non-numeric.
|
||||
// Returns 0 if the suffix cannot be parsed as a uint64 (Windows fallback).
|
||||
func inoFromToken(token string) uint64 {
|
||||
dot := strings.Index(token, ".")
|
||||
if dot < 0 {
|
||||
return 0
|
||||
}
|
||||
ino, err := strconv.ParseUint(token[dot+1:], 10, 64)
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
return ino
|
||||
}
|
||||
|
||||
// CommitMetadata moves the per-upload temporary sidecar attributes written by
|
||||
// StoreAttribute to the final location for (bucket, object). It must be
|
||||
// called after the data file has been linked into the namespace.
|
||||
//
|
||||
// token is the per-upload sidecar identifier returned by tmpfile.SidecarToken(),
|
||||
// computed before link() closed the file descriptor.
|
||||
//
|
||||
// dataPath is the filesystem path of the committed data file (e.g.
|
||||
// filepath.Join(bucket, object) from the POSIX backend). It is used to
|
||||
// re-verify the inode before each attribute rename so that a goroutine whose
|
||||
// inode was subsequently displaced by a later link() aborts instead of
|
||||
// overwriting the correct winner's metadata.
|
||||
//
|
||||
// Each attribute file is moved individually (atomic rename). Before each
|
||||
// rename the inode at dataPath is re-verified against the inode encoded in
|
||||
// token. If the inode no longer matches (a later concurrent link() has
|
||||
// installed a different upload's data file), this goroutine is no longer the
|
||||
// winner: it aborts, removes its staged temp directory, and returns without
|
||||
// error. The true winner's CommitMetadata will overwrite any partially
|
||||
// committed attributes before it finishes, leaving only consistent metadata.
|
||||
func (s SideCar) CommitMetadata(bucket, object, token, dataPath string) error {
|
||||
if token == "" || object == "" {
|
||||
return nil
|
||||
}
|
||||
|
||||
myIno := inoFromToken(token)
|
||||
|
||||
tempDir := filepath.Join(s.dir, bucket, ".sgwtmp."+token)
|
||||
tempMetaDir := filepath.Join(tempDir, sidecarmeta)
|
||||
finalMetaDir := filepath.Join(s.dir, bucket, object, sidecarmeta)
|
||||
|
||||
entries, err := os.ReadDir(tempMetaDir)
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
// No metadata was staged for this upload; nothing to commit.
|
||||
return nil
|
||||
}
|
||||
if err != nil {
|
||||
return fmt.Errorf("read staged metadata: %w", err)
|
||||
}
|
||||
|
||||
if err := os.MkdirAll(finalMetaDir, 0777); err != nil {
|
||||
if errors.Is(err, syscall.ENOSPC) {
|
||||
return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice)
|
||||
}
|
||||
return fmt.Errorf("create final metadata dir: %w", err)
|
||||
}
|
||||
|
||||
for _, ent := range entries {
|
||||
// Re-verify we are still the link() winner before each rename.
|
||||
// If a concurrent goroutine has since won link() (replacing our inode
|
||||
// at dataPath), abort now. Any attributes we already renamed will be
|
||||
// overwritten by the true winner's CommitMetadata, which will run to
|
||||
// completion because no later link() can displace its inode.
|
||||
if myIno != 0 && pathIno(dataPath) != myIno {
|
||||
os.RemoveAll(tempDir)
|
||||
return nil
|
||||
}
|
||||
|
||||
src := filepath.Join(tempMetaDir, ent.Name())
|
||||
dst := filepath.Join(finalMetaDir, ent.Name())
|
||||
if err := os.Rename(src, dst); err != nil && !errors.Is(err, os.ErrNotExist) {
|
||||
return fmt.Errorf("commit attribute %s: %w", ent.Name(), err)
|
||||
}
|
||||
}
|
||||
|
||||
os.RemoveAll(tempDir)
|
||||
return nil
|
||||
}
|
||||
|
||||
// CleanupMetadata removes the per-upload temporary sidecar directory staged by
|
||||
// StoreAttribute without promoting it to the final location. It should be
|
||||
// called when the upload lost the concurrent link() race or when link()
|
||||
// returned EEXIST, to prevent orphaned .sgwtmp.* directories from accumulating.
|
||||
func (s SideCar) CleanupMetadata(bucket, token string) error {
|
||||
if token == "" {
|
||||
return nil
|
||||
}
|
||||
tempDir := filepath.Join(s.dir, bucket, ".sgwtmp."+token)
|
||||
if err := os.RemoveAll(tempDir); err != nil && !errors.Is(err, os.ErrNotExist) {
|
||||
return fmt.Errorf("cleanup staged metadata: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// RenameObject renames the sidecar metadata directory from oldObject to
|
||||
// newObject so that path-based lookups continue to work after the data
|
||||
// directory has been renamed.
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
// Copyright 2026 Versity Software
|
||||
// This file is licensed under the Apache License, Version 2.0
|
||||
// (the "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing,
|
||||
// software distributed under the License is distributed on an
|
||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
// KIND, either express or implied. See the License for the
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
|
||||
//go:build !windows
|
||||
|
||||
package meta
|
||||
|
||||
import (
|
||||
"os"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
// fileIno returns the inode number of the open file f. The inode is a
|
||||
// stable, filesystem-unique identifier for the file's data, making it safe
|
||||
// to use as part of a per-upload sidecar directory name. Returns 0 on error.
|
||||
func fileIno(f *os.File) uint64 {
|
||||
fi, err := f.Stat()
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
st, ok := fi.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
return 0
|
||||
}
|
||||
return st.Ino
|
||||
}
|
||||
|
||||
// pathIno returns the inode number of the file at path by calling Lstat.
|
||||
// Used by CommitMetadata to verify the object path still holds this upload's
|
||||
// inode before each attribute rename. Returns 0 on error.
|
||||
func pathIno(path string) uint64 {
|
||||
fi, err := os.Lstat(path)
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
st, ok := fi.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
return 0
|
||||
}
|
||||
return st.Ino
|
||||
}
|
||||
@@ -12,14 +12,21 @@
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
|
||||
//go:build !windows
|
||||
//go:build windows
|
||||
|
||||
package posix
|
||||
package meta
|
||||
|
||||
import (
|
||||
"github.com/versity/versitygw/s3err"
|
||||
)
|
||||
import "os"
|
||||
|
||||
func handleParentDirError(_ string) error {
|
||||
return s3err.GetAPIError(s3err.ErrObjectParentIsFile)
|
||||
// fileIno returns 0 on Windows because Windows does not expose POSIX inodes
|
||||
// via the standard syscall interface. tmpSidecarID falls back to the
|
||||
// path-based identifier, which is always unique on Windows because
|
||||
// CreateTemp generates uniquely-named files.
|
||||
func fileIno(_ *os.File) uint64 {
|
||||
return 0
|
||||
}
|
||||
|
||||
// pathIno returns 0 on Windows; POSIX inodes are not available.
|
||||
func pathIno(_ string) uint64 {
|
||||
return 0
|
||||
}
|
||||
@@ -91,6 +91,18 @@ func (x XattrMeta) DeleteAttributes(bucket, object string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// CommitMetadata is a no-op for xattr: extended attributes are stored on the
|
||||
// inode and travel atomically with the data file through link(), so there is
|
||||
// no temporary staging area to commit.
|
||||
func (x XattrMeta) CommitMetadata(_, _, _, _ string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// CleanupMetadata is a no-op for xattr: there is no temporary staging area.
|
||||
func (x XattrMeta) CleanupMetadata(_, _ string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// RenameObject is a no-op for xattr because extended attributes are stored
|
||||
// on the inodes and follow the file/directory when it is renamed.
|
||||
func (x XattrMeta) RenameObject(_, _, _ string) error {
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
// Copyright 2026 Versity Software
|
||||
// This file is licensed under the Apache License, Version 2.0
|
||||
// (the "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing,
|
||||
// software distributed under the License is distributed on an
|
||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
// KIND, either express or implied. See the License for the
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
|
||||
//go:build !windows
|
||||
|
||||
package posix
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"syscall"
|
||||
|
||||
"github.com/versity/versitygw/s3err"
|
||||
)
|
||||
|
||||
func handleParentDirError(_ string) error {
|
||||
return s3err.GetAPIError(s3err.ErrObjectParentIsFile)
|
||||
}
|
||||
|
||||
// captureIno returns the inode number of the open file f by calling fstat.
|
||||
// This is used to capture the inode of a temp upload file while its fd is
|
||||
// still open, before link() closes it. The captured inode is stored in
|
||||
// tmpfile.ino and used later by SidecarToken() and didWinLink().
|
||||
func captureIno(f *os.File) uint64 {
|
||||
fi, err := f.Stat()
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
st, ok := fi.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
return 0
|
||||
}
|
||||
return st.Ino
|
||||
}
|
||||
|
||||
// didWinLink reports whether this upload's data file is the one currently
|
||||
// installed at the final object path. It compares the inode captured just
|
||||
// before link() closed the temp file against the inode of the live object.
|
||||
// Only the winner should call CommitMetadata.
|
||||
//
|
||||
// If ino is 0 (not captured, or on an error path) this returns false so that
|
||||
// metadata commit is skipped rather than potentially committing stale data.
|
||||
func (tmp *tmpfile) didWinLink() bool {
|
||||
if tmp.ino == 0 {
|
||||
return false
|
||||
}
|
||||
fi, err := os.Lstat(filepath.Join(tmp.bucket, tmp.objname))
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
st, ok := fi.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
return st.Ino == tmp.ino
|
||||
}
|
||||
@@ -44,3 +44,17 @@ func handleParentDirError(name string) error {
|
||||
// Parent doesn't exist or is a directory, treat as ENOENT
|
||||
return nil
|
||||
}
|
||||
|
||||
// captureIno returns 0 on Windows; POSIX inodes are not available.
|
||||
// Windows uploads always use CreateTemp which produces uniquely-named files,
|
||||
// so path-based sidecar tokens are safe there.
|
||||
func captureIno(_ *os.File) uint64 {
|
||||
return 0
|
||||
}
|
||||
|
||||
// didWinLink always returns true on Windows. Windows uploads use CreateTemp
|
||||
// which produces uniquely-named files, so there is no fd-reuse race and no
|
||||
// concurrent winner determination is needed.
|
||||
func (tmp *tmpfile) didWinLink() bool {
|
||||
return true
|
||||
}
|
||||
@@ -2266,6 +2266,15 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C
|
||||
return res, "", fmt.Errorf("link object in namespace: %w", err)
|
||||
}
|
||||
|
||||
if f.didWinLink() {
|
||||
if err := p.meta.CommitMetadata(bucket, object, f.SidecarToken(), filepath.Join(bucket, object)); err != nil {
|
||||
return res, "", fmt.Errorf("commit object metadata: %w", err)
|
||||
}
|
||||
} else {
|
||||
// This upload lost the concurrent link() race; clean up its staged sidecar.
|
||||
_ = p.meta.CleanupMetadata(bucket, f.SidecarToken())
|
||||
}
|
||||
|
||||
// cleanup tmp dirs
|
||||
os.RemoveAll(filepath.Join(bucket, objdir, activeUploadName))
|
||||
// use Remove for objdir in case there are still other uploads
|
||||
@@ -3251,6 +3260,15 @@ func (p *Posix) UploadPartWithPostFunc(ctx context.Context, input *s3.UploadPart
|
||||
return nil, fmt.Errorf("link object in namespace: %w", err)
|
||||
}
|
||||
|
||||
if f.didWinLink() {
|
||||
if err := p.meta.CommitMetadata(bucket, partPath, f.SidecarToken(), filepath.Join(bucket, partPath)); err != nil {
|
||||
return nil, fmt.Errorf("commit part metadata: %w", err)
|
||||
}
|
||||
} else {
|
||||
// This upload lost the concurrent link() race; clean up its staged sidecar.
|
||||
_ = p.meta.CleanupMetadata(bucket, f.SidecarToken())
|
||||
}
|
||||
|
||||
return res, nil
|
||||
}
|
||||
|
||||
@@ -3509,6 +3527,15 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput)
|
||||
return s3response.CopyPartResult{}, fmt.Errorf("link object in namespace: %w", err)
|
||||
}
|
||||
|
||||
if f.didWinLink() {
|
||||
if err := p.meta.CommitMetadata(*upi.Bucket, partPath, f.SidecarToken(), filepath.Join(*upi.Bucket, partPath)); err != nil {
|
||||
return s3response.CopyPartResult{}, fmt.Errorf("commit part metadata: %w", err)
|
||||
}
|
||||
} else {
|
||||
// This upload lost the concurrent link() race; clean up its staged sidecar.
|
||||
_ = p.meta.CleanupMetadata(*upi.Bucket, f.SidecarToken())
|
||||
}
|
||||
|
||||
fi, err = os.Stat(filepath.Join(*upi.Bucket, partPath))
|
||||
if err != nil {
|
||||
return s3response.CopyPartResult{}, fmt.Errorf("stat part path: %w", err)
|
||||
@@ -3929,6 +3956,9 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje
|
||||
|
||||
err = f.link()
|
||||
if errors.Is(err, syscall.EEXIST) {
|
||||
// Another upload installed its data file first; discard our staged
|
||||
// sidecar metadata to prevent .sgwtmp.* dirs from accumulating.
|
||||
_ = p.meta.CleanupMetadata(*po.Bucket, f.SidecarToken())
|
||||
return s3response.PutObjectOutput{
|
||||
ETag: etag,
|
||||
VersionID: versionID,
|
||||
@@ -3938,6 +3968,15 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje
|
||||
return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrExistingObjectIsDirectory)
|
||||
}
|
||||
|
||||
if f.didWinLink() {
|
||||
if err := p.meta.CommitMetadata(*po.Bucket, *po.Key, f.SidecarToken(), filepath.Join(*po.Bucket, *po.Key)); err != nil {
|
||||
return s3response.PutObjectOutput{}, fmt.Errorf("commit object metadata: %w", err)
|
||||
}
|
||||
} else {
|
||||
// This upload lost the concurrent link() race; clean up its staged sidecar.
|
||||
_ = p.meta.CleanupMetadata(*po.Bucket, f.SidecarToken())
|
||||
}
|
||||
|
||||
// Set object tagging
|
||||
if tags != nil {
|
||||
err := p.PutObjectTagging(withCtxNoSlot(ctx), *po.Bucket, *po.Key, "", tags)
|
||||
|
||||
@@ -18,6 +18,7 @@ import (
|
||||
"fmt"
|
||||
"math/rand"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -35,6 +36,28 @@ func (tmp *tmpfile) File() *os.File {
|
||||
return tmp.f
|
||||
}
|
||||
|
||||
// SidecarToken returns the per-upload identifier used to name the temporary
|
||||
// sidecar directory staged by StoreAttribute. It must be called after
|
||||
// link() so that tmp.ino has been set.
|
||||
//
|
||||
// On Unix the token is "<pid>.<inode>", which is unique for the lifetime of
|
||||
// the upload's data file regardless of fd number reuse.
|
||||
// On platforms where inodes are unavailable (e.g. Windows) it falls back to
|
||||
// "<pid>.<basename(f.Name())>", which is unique because CreateTemp generates
|
||||
// uniquely-named files.
|
||||
//
|
||||
// IMPORTANT: the token format must stay in sync with tmpSidecarID() in
|
||||
// backend/meta/sidecar.go, which computes the same value from the live *os.File
|
||||
// during StoreAttribute. Both functions must produce identical output for the
|
||||
// staging and commit steps to find the same directory.
|
||||
func (tmp *tmpfile) SidecarToken() string {
|
||||
if tmp.ino != 0 {
|
||||
return fmt.Sprintf("%d.%d", os.Getpid(), tmp.ino)
|
||||
}
|
||||
// Fallback: path-based token for platforms without POSIX inodes.
|
||||
return fmt.Sprintf("%d.%s", os.Getpid(), filepath.Base(tmp.f.Name()))
|
||||
}
|
||||
|
||||
func sleepWithJitter(backoffMs int) {
|
||||
if backoffMs <= 1 {
|
||||
time.Sleep(1 * time.Millisecond)
|
||||
@@ -13,7 +13,6 @@
|
||||
// under the License.
|
||||
|
||||
//go:build linux
|
||||
// +build linux
|
||||
|
||||
package posix
|
||||
|
||||
@@ -47,6 +46,10 @@ type tmpfile struct {
|
||||
uid int
|
||||
gid int
|
||||
newDirPerm fs.FileMode
|
||||
// ino holds the inode of the temp file captured just before link() renames
|
||||
// it into place. Used by didWinLink() to determine whether this upload is
|
||||
// the one currently installed at the final object path.
|
||||
ino uint64
|
||||
}
|
||||
|
||||
var (
|
||||
@@ -217,6 +220,10 @@ func (tmp *tmpfile) link() error {
|
||||
return tmp.fallbackLink()
|
||||
}
|
||||
|
||||
// Capture the inode via fstat before linkat so didWinLink() can later
|
||||
// verify that this upload's file is the one at the final path.
|
||||
tmp.ino = captureIno(tmp.f)
|
||||
|
||||
procdir, err := os.Open(procfddir)
|
||||
if err != nil {
|
||||
return fmt.Errorf("open proc dir: %w", err)
|
||||
@@ -270,6 +277,10 @@ func (tmp *tmpfile) link() error {
|
||||
func (tmp *tmpfile) fallbackLink() error {
|
||||
tempname := tmp.f.Name()
|
||||
|
||||
// Capture the inode before closing/renaming so didWinLink() can later
|
||||
// verify that this upload's file is the one at the final path.
|
||||
tmp.ino = captureIno(tmp.f)
|
||||
|
||||
// reset default file mode because CreateTemp uses 0600
|
||||
tmp.f.Chmod(fs.FileMode(defaultFilePerm))
|
||||
|
||||
|
||||
@@ -44,6 +44,10 @@ type tmpfile struct {
|
||||
uid int
|
||||
gid int
|
||||
doChown bool
|
||||
// ino holds the inode of the temp file captured just before link() renames
|
||||
// it into place. Used by didWinLink() to determine whether this upload is
|
||||
// the one currently installed at the final object path.
|
||||
ino uint64
|
||||
}
|
||||
|
||||
func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Account, _ bool, _ bool) (*tmpfile, error) {
|
||||
@@ -98,6 +102,10 @@ func (tmp *tmpfile) link() error {
|
||||
|
||||
objPath := filepath.Join(tmp.bucket, tmp.objname)
|
||||
|
||||
// Capture the inode before closing/renaming so didWinLink() can later
|
||||
// verify that this upload's file is the one at the final path.
|
||||
tmp.ino = captureIno(tmp.f)
|
||||
|
||||
// reset default file mode because CreateTemp uses 0600
|
||||
tmp.f.Chmod(defaultFilePerm)
|
||||
|
||||
|
||||
@@ -564,7 +564,7 @@ func TestCompleteMultipartUpload(ts *TestState) {
|
||||
}
|
||||
ts.Run(CompleteMultipartUpload_success)
|
||||
ts.Run(CompleteMultipartUpload_already_completed)
|
||||
if !(ts.conf.azureTests || ts.conf.sidecarTests) {
|
||||
if !(ts.conf.azureTests) {
|
||||
ts.Run(CompleteMultipartUpload_racey_success)
|
||||
ts.Run(CompleteMultipartUpload_racey_data_integrity)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user