core: revise chunk manifest loading (completed vs partial)

* add LoadCompleted() and LoadPartial() methods
  - remove fallback logic from file I/O - callers now specify intent explicitly
* add cos.ValidateManifestID() for consistent upload ID validation
* amend packed size estimation
* update tests to use revised APIs
* with refactoring and cleanup
----
* part thirteen, prev. commit: 9bad95b

Signed-off-by: Alex Aizman <alex.aizman@gmail.com>

Author: alex-aizman

ais/s3/mpt.go

@@ -19,11 +19,12 @@ func ListUploads(all []*core.Ufest, bckName, idMarker string, maxUploads int) *L
 
 	// filter by bucket
 	for _, manifest := range all {
-		if bckName == "" || manifest.Lom.Bck().Name == bckName {
+		lom := manifest.Lom()
+		if bckName == "" || lom.Bck().Name == bckName {
 			results = append(results, UploadInfoResult{
-				Key:       manifest.Lom.ObjName,
+				Key:       lom.ObjName,
 				UploadID:  manifest.ID,
-				Initiated: manifest.Created,
+				Initiated: manifest.Created(),
 			})
 		}
 	}

ais/tgtmpt.go

@@ -50,9 +50,9 @@ func (ups *ups) init(id string, lom *core.LOM, rmd map[string]string) error {
 }
 
 func (ups *ups) _add(id string, manifest *core.Ufest, rmd map[string]string) {
-	debug.Assert(manifest.Lom != nil)
+	debug.Assert(manifest.Lom() != nil)
 	_, ok := ups.m[id]
-	debug.Assert(!ok, "duplicated upload ID: ", id, " ", manifest.Lom.Cname())
+	debug.Assert(!ok, "duplicated upload ID: ", id, " ", manifest.Lom().Cname())
 	ups.m[id] = up{manifest, rmd}
 }
 
@@ -72,13 +72,13 @@ func (ups *ups) getWithMeta(id string) (manifest *core.Ufest, rmd map[string]str
 }
 
 // NOTE: must be called with ups and lom both unlocked
-func (ups *ups) fromFS(id string, lom *core.LOM, add bool) (manifest *core.Ufest, err error) {
+func (ups *ups) loadPartial(id string, lom *core.LOM, add bool) (manifest *core.Ufest, err error) {
 	debug.Assert(lom.IsLocked() == apc.LockNone, "expecting not locked: ", lom.Cname())
 
 	lom.Lock(false)
 	defer lom.Unlock(false)
 	manifest = core.NewUfest(id, lom, true /*must-exist*/)
-	if err = manifest.Load(lom); err == nil && add {
+	if err = manifest.LoadPartial(lom); err == nil && add {
 		ups.Lock()
 		ups._add(id, manifest, nil /*remote metadata <= LOM custom*/)
 		ups.Unlock()
@@ -92,7 +92,7 @@ func (ups *ups) abort(id string, lom *core.LOM) (ecode int, err error) {
 	up, ok := ups.m[id]
 	if !ok {
 		ups.Unlock()
-		manifest, err = ups.fromFS(id, lom, false /*add*/)
+		manifest, err = ups.loadPartial(id, lom, false /*add*/)
 		if err != nil {
 			return 0, err
 		}

ais/tgts3mpt.go

@@ -4,42 +4,17 @@
  */
 package ais
 
-// NOTE -- TODO: S3 Multipart Upload Implementation
-//
 // 1. in-memory state
 //    - active uploads kept purely in-memory (ups map)
 //    - no support for target restart recovery during active uploads
-//    - if target restarts during upload, client must restart the entire upload
-//    - this simplifies design and avoids complex state recovery mechanisms
-//    - rationale: most multipart uploads complete within hours; target restarts
-//      are rare operational events. the complexity of persistent state recovery
-//      outweighs the benefit for the typical use case.
-//
-// 2. cleanup
-//    - no automatic cleanup of abandoned uploads
-//    - uploads remain in memory until explicitly completed or aborted
-//
-// 3. parts ordering
-//    - completeMpt enforces strictly ascending, contiguous parts (1..n)
-//    - this is stricter than aws s3, which allows gaps and sparse part sets
-//    - rationale: simplifies chunk manifest structure and append logic;
-//      most s3 clients upload parts sequentially anyway
+//      (if target restarts during upload, client must restart the entire upload)
 //
-// 4. concurrency
-//    - global rwmutex for upload cache operations
-//    - individual manifest mutexes for chunk operations
-//    - tradeoff: simple consistency vs potential lock contention at scale
+// 2. t.completeMpt() locks/unlocks two times - consider CoW
 //
-// 5. error handling
-//    - fail fast on inconsistencies (size mismatches, missing parts)
-//    - atomic operations where possible (chunk replacement, manifest storage)
-//    - orphaned chunks cleaned up only on explicit abort, not on errors
+// 3. TODO cleanup; orphan chunks, abandoned (partial) manifests
 //
-// 6. persistence
-//    - chunk manifests persisted to xattr only after successful completion
-//    - used later for individual chunk access (getMptPart)
-//    - not used for active upload state recovery
-//    - failed/aborted uploads clean up chunks but don't persist manifests
+// 4. parts ordering
+//    - Add(chunk) keeps chunks ("parts") sorted
 
 import (
 	"errors"
@@ -246,7 +221,7 @@ func (t *target) putMptPart(w http.ResponseWriter, r *http.Request, items []stri
 		debug.Assert(len(chunk.MD5) == 16, len(chunk.MD5))
 	}
 	if checkPartSHA {
-		chunk.Cksum = &cksumSHA.Cksum
+		chunk.SetCksum(&cksumSHA.Cksum)
 	}
 
 	// - see NOTE above in re "active uploads in memory"
@@ -498,7 +473,7 @@ func (t *target) listMptParts(w http.ResponseWriter, r *http.Request, bck *meta.
 	manifest := t.ups.get(uploadID)
 	if manifest == nil {
 		var err error
-		if manifest, err = t.ups.fromFS(uploadID, lom, true /*add*/); err != nil {
+		if manifest, err = t.ups.loadPartial(uploadID, lom, true /*add*/); err != nil {
 			s3.WriteMptErr(w, r, s3.NewErrNoSuchUpload(uploadID, err), http.StatusNotFound, lom, uploadID)
 			return
 		}
@@ -563,7 +538,7 @@ func (t *target) getMptPart(w http.ResponseWriter, r *http.Request, bck *meta.Bc
 
 	// load chunk manifest and find out the part num's offset & size
 	manifest := core.NewUfest("", lom, true /*must-exist*/)
-	err = manifest.Load(lom)
+	err = manifest.LoadCompleted(lom)
 	if err != nil {
 		s3.WriteErr(w, r, err, http.StatusNotFound)
 		return

cmn/cos/uuid.go

@@ -124,6 +124,31 @@ func GenTestingDaemonID(suffix string) string {
 	return CryptoRandS(l) + suffix
 }
 
+//
+// chunk manifest ID
+//
+
+func ValidateManifestID(id string) error {
+	const (
+		lmin = 8
+		lmax = 128
+	)
+	l := len(id)
+	if l < lmin {
+		return fmt.Errorf("chunk manifest ID %q is too short (expecting >= %d)", id, lmin)
+	}
+	if l > lmax {
+		return fmt.Errorf("chunk manifest ID %q is too long (expecting <= %d)", id, lmax)
+	}
+	for i := range l {
+		c := id[i]
+		if c < 32 || c > 126 || c == '/' || c == '\\' {
+			return fmt.Errorf("chunk manifest ID %q contains invalid character at position %d (expecting ASCII)", id, i)
+		}
+	}
+	return nil
+}
+
 //
 // utility functions
 //