From 24722ddd02b6b37d28390c98d9f5a5f8124d9f2a Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Fri, 13 Aug 2021 17:25:54 +0200 Subject: [PATCH] Remove inline data hack (#12946) move the code down to the storage layer, this logic decouples the inline data from the size parameter making it flexible and future proof. --- cmd/erasure-object.go | 20 -------------------- cmd/storage-datatypes.go | 5 ++++- cmd/xl-storage-format-v2.go | 2 +- cmd/xl-storage.go | 27 ++++++++++++++++++++++----- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go index 3d360f3d4..c2d11aad1 100644 --- a/cmd/erasure-object.go +++ b/cmd/erasure-object.go @@ -256,26 +256,6 @@ func (er erasureObjects) getObjectWithFileInfo(ctx context.Context, bucket, obje return toObjectErr(err, bucket, object) } - // This hack is needed to avoid a bug found when overwriting - // the inline data object with a un-inlined version, for the - // time being we need this as we have released inline-data - // version already and this bug is already present in newer - // releases. - // - // This mainly happens with objects < smallFileThreshold when - // they are overwritten with un-inlined objects >= smallFileThreshold, - // due to a bug in RenameData() the fi.Data is not niled leading to - // GetObject thinking that fi.Data is valid while fi.Size has - // changed already. - if fi.InlineData() { - shardFileSize := erasure.ShardFileSize(fi.Size) - if shardFileSize >= 0 && shardFileSize >= smallFileThreshold { - for i := range metaArr { - metaArr[i].Data = nil - } - } - } - var healOnce sync.Once // once we have obtained a common FileInfo i.e latest, we should stick diff --git a/cmd/storage-datatypes.go b/cmd/storage-datatypes.go index 94d21eea1..76e314a51 100644 --- a/cmd/storage-datatypes.go +++ b/cmd/storage-datatypes.go @@ -190,7 +190,10 @@ func (fi FileInfo) InlineData() bool { } // SetInlineData marks object (version) as inline. -func (fi FileInfo) SetInlineData() { +func (fi *FileInfo) SetInlineData() { + if fi.Metadata == nil { + fi.Metadata = make(map[string]string, 1) + } fi.Metadata[ReservedMetadataPrefixLower+"inline-data"] = "true" } diff --git a/cmd/xl-storage-format-v2.go b/cmd/xl-storage-format-v2.go index f51137da8..088739124 100644 --- a/cmd/xl-storage-format-v2.go +++ b/cmd/xl-storage-format-v2.go @@ -253,7 +253,7 @@ type xlMetaV2 struct { //msgp:ignore xlMetaInlineData type xlMetaInlineData []byte -// xlMetaInlineDataVer indicates the vesrion of the inline data structure. +// xlMetaInlineDataVer indicates the version of the inline data structure. const xlMetaInlineDataVer = 1 // versionOK returns whether the version is ok. diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index 0d149a959..a818408ee 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -1130,12 +1130,29 @@ func (s *xlStorage) ReadVersion(ctx context.Context, volume, path, versionID str if readData { if len(fi.Data) > 0 || fi.Size == 0 { - if len(fi.Data) > 0 { - if !fi.InlineData() { - fi.SetInlineData() - } + if fi.InlineData() { + // If written with header we are fine. + return fi, nil } - return fi, nil + if fi.Size == 0 || !(fi.VersionID != "" && fi.VersionID != nullVersionID) { + // If versioned we have no conflicts. + fi.SetInlineData() + return fi, nil + } + + // For overwritten objects without header we might have a conflict with + // data written later. + // Check the data path if there is a part with data. + partPath := fmt.Sprintf("part.%d", fi.Parts[0].Number) + dataPath := pathJoin(path, fi.DataDir, partPath) + _, err = s.StatInfoFile(ctx, volume, dataPath) + if err != nil { + // Set the inline header, our inlined data is fine. + fi.SetInlineData() + return fi, nil + } + // Data exists on disk, remove the version from metadata. + fi.Data = nil } // Reading data for small objects when