Skip to content

Commit cfb660f

Browse files
committed
storage: fix tomstoned objects revival not working
Delete tombstone after revival. Note that deletion occurs only on the specific node where the object is located. Closes #3486. Signed-off-by: Andrey Butusov <[email protected]>
1 parent 2d36e46 commit cfb660f

File tree

7 files changed

+191
-3
lines changed

7 files changed

+191
-3
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Changelog for NeoFS Node
77

88
### Fixed
99
- Send on closed channel panic in node's new epoch handler (#3529)
10+
- Tomstoned objects revival not working (#3542)
1011

1112
### Changed
1213
- Stream payload without buffering to reduce memory usage in CLI `Get/Put` operations (#3535)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package engine
2+
3+
import (
4+
"testing"
5+
6+
"github.com/nspcc-dev/neofs-node/pkg/core/object"
7+
meta "github.com/nspcc-dev/neofs-node/pkg/local_object_storage/metabase"
8+
apistatus "github.com/nspcc-dev/neofs-sdk-go/client/status"
9+
cidtest "github.com/nspcc-dev/neofs-sdk-go/container/id/test"
10+
objectSDK "github.com/nspcc-dev/neofs-sdk-go/object"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestStorageEngine_ReviveObject(t *testing.T) {
15+
e := testNewEngineWithShardNum(t, 1)
16+
defer func() { _ = e.Close() }()
17+
18+
cnr := cidtest.ID()
19+
20+
obj := generateObjectWithCID(cnr)
21+
require.NoError(t, e.Put(obj, nil))
22+
addr := object.AddressOf(obj)
23+
24+
ts := generateObjectWithCID(cnr)
25+
ts.SetType(objectSDK.TypeTombstone)
26+
addAttribute(ts, objectSDK.AttributeAssociatedObject, obj.GetID().EncodeToString())
27+
addAttribute(ts, objectSDK.AttributeExpirationEpoch, "0")
28+
tsAddr := object.AddressOf(ts)
29+
30+
require.NoError(t, e.Put(ts, nil))
31+
32+
_, err := e.Get(addr)
33+
require.ErrorIs(t, err, apistatus.ErrObjectAlreadyRemoved)
34+
35+
rs, err := e.ReviveObject(addr)
36+
require.NoError(t, err)
37+
require.Equal(t, meta.ReviveStatusGraveyard, rs.Shards[0].Status.StatusType())
38+
require.Equal(t, tsAddr, rs.Shards[0].Status.TombstoneAddress())
39+
40+
got, err := e.Get(addr)
41+
require.NoError(t, err)
42+
require.Equal(t, obj, got)
43+
44+
// tombstone metadata should be dropped, so getting TS must fail with not found
45+
_, err = e.Get(tsAddr)
46+
require.ErrorIs(t, err, apistatus.ErrObjectNotFound)
47+
}

pkg/local_object_storage/metabase/put_test.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,28 @@ func TestDB_Put_ObjectWithTombstone(t *testing.T) {
167167
require.NoError(t, err)
168168
require.Equal(t, meta.ReviveStatusGarbage, rs.StatusType())
169169
require.Equal(t, "successful revival from garbage bucket", rs.Message())
170+
require.Equal(t, tsAddr, rs.TombstoneAddress())
170171

171172
t.Run("after revival", func(t *testing.T) {
172-
t.Skip("https://github.com/nspcc-dev/neofs-node/issues/3486")
173+
// tombstone is still there, but the garbage was cleared
174+
t.Run("get garbage", func(t *testing.T) {
175+
gObjs, _, err := db.GetGarbage(100)
176+
require.NoError(t, err)
177+
require.NotContains(t, gObjs, addr)
178+
})
179+
t.Run("iterate garbage", func(t *testing.T) {
180+
var collected []oid.Address
181+
err := db.IterateOverGarbage(func(gObj meta.GarbageObject) error {
182+
collected = append(collected, gObj.Address())
183+
return nil
184+
}, nil)
185+
require.NoError(t, err)
186+
require.NotContains(t, collected, addr)
187+
})
188+
189+
_, err = db.Delete([]oid.Address{tsAddr})
190+
require.NoError(t, err)
191+
173192
assertObjectAvailability(t, db, addr, obj)
174193
})
175194
}

pkg/local_object_storage/metabase/revive.go

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package meta
22

33
import (
4+
"bytes"
45
"fmt"
56

67
"github.com/nspcc-dev/neofs-node/pkg/local_object_storage/util/logicerr"
@@ -31,6 +32,8 @@ const (
3132
type ReviveStatus struct {
3233
statusType reviveStatusType
3334
message string
35+
// tombstoneAddr holds the address of the tombstone used to inhume the object (if any).
36+
tombstoneAddr oid.Address
3437
}
3538

3639
// Message returns message of status.
@@ -43,6 +46,11 @@ func (s *ReviveStatus) StatusType() reviveStatusType {
4346
return s.statusType
4447
}
4548

49+
// TombstoneAddress returns the tombstone address.
50+
func (s *ReviveStatus) TombstoneAddress() oid.Address {
51+
return s.tombstoneAddr
52+
}
53+
4654
func (s *ReviveStatus) setStatusGraveyard(tomb string) {
4755
s.statusType = ReviveStatusGraveyard
4856
s.message = fmt.Sprintf("successful revival from graveyard, tomb: %s", tomb)
@@ -73,6 +81,7 @@ func (db *DB) ReviveObject(addr oid.Address) (res ReviveStatus, err error) {
7381
}
7482

7583
currEpoch := db.epochState.CurrentEpoch()
84+
cnr := addr.Container()
7685

7786
err = db.boltDB.Update(func(tx *bbolt.Tx) error {
7887
garbageObjectsBKT := tx.Bucket(garbageObjectsBucketName)
@@ -101,6 +110,7 @@ func (db *DB) ReviveObject(addr oid.Address) (res ReviveStatus, err error) {
101110
return err
102111
}
103112
res.setStatusGraveyard(tombAddress.EncodeToString())
113+
res.tombstoneAddr = tombAddress
104114
} else {
105115
val = garbageContainersBKT.Get(targetKey[:cidSize])
106116
if val != nil {
@@ -116,6 +126,20 @@ func (db *DB) ReviveObject(addr oid.Address) (res ReviveStatus, err error) {
116126
// nor was marked with GC mark
117127
return ErrObjectWasNotRemoved
118128
}
129+
130+
metaBucket := tx.Bucket(metaBucketKey(cnr))
131+
var metaCursor *bbolt.Cursor
132+
if metaBucket != nil {
133+
metaCursor = metaBucket.Cursor()
134+
}
135+
136+
tombID, err := getTombstoneByAssociatedObject(metaCursor, addr.Object())
137+
if err != nil {
138+
return fmt.Errorf("iterate covered by tombstones: %w", err)
139+
}
140+
if !tombID.IsZero() {
141+
res.tombstoneAddr = oid.NewAddress(cnr, tombID)
142+
}
119143
}
120144

121145
if err := garbageObjectsBKT.Delete(targetKey); err != nil {
@@ -126,7 +150,7 @@ func (db *DB) ReviveObject(addr oid.Address) (res ReviveStatus, err error) {
126150
// if object is stored, and it is regular object then update bucket
127151
// with container size estimations
128152
if obj.Type() == object.TypeRegular {
129-
if err := changeContainerInfo(tx, addr.Container(), int(obj.PayloadSize()), 1); err != nil {
153+
if err := changeContainerInfo(tx, cnr, int(obj.PayloadSize()), 1); err != nil {
130154
return err
131155
}
132156
}
@@ -145,3 +169,42 @@ func (db *DB) ReviveObject(addr oid.Address) (res ReviveStatus, err error) {
145169

146170
return
147171
}
172+
173+
// getTombstoneByAssociatedObject iterates meta index to find tombstone object
174+
// associated with the provided object ID. If found, returns its ID, otherwise
175+
// returns zero ID and nil error.
176+
func getTombstoneByAssociatedObject(metaCursor *bbolt.Cursor, idObj oid.ID) (oid.ID, error) {
177+
var id oid.ID
178+
if metaCursor == nil {
179+
return id, fmt.Errorf("nil meta cursor")
180+
}
181+
182+
var (
183+
typString = object.TypeTombstone.String()
184+
idStr = idObj.EncodeToString()
185+
accPrefix = make([]byte, 1+len(object.AttributeAssociatedObject)+1+len(idStr)+1)
186+
typeKey = make([]byte, metaIDTypePrefixSize+len(typString))
187+
expirationPrefix = make([]byte, attrIDFixedLen+len(object.AttributeExpirationEpoch))
188+
)
189+
190+
expirationPrefix[0] = metaPrefixIDAttr
191+
copy(expirationPrefix[1+oid.Size:], object.AttributeExpirationEpoch)
192+
193+
accPrefix[0] = metaPrefixAttrIDPlain
194+
copy(accPrefix[1:], object.AttributeAssociatedObject)
195+
copy(accPrefix[1+len(object.AttributeAssociatedObject)+1:], idStr)
196+
197+
fillIDTypePrefix(typeKey)
198+
copy(typeKey[metaIDTypePrefixSize:], typString)
199+
200+
for k, _ := metaCursor.Seek(accPrefix); bytes.HasPrefix(k, accPrefix); k, _ = metaCursor.Next() {
201+
mainObj := k[len(accPrefix):]
202+
copy(typeKey[1:], mainObj)
203+
204+
if metaCursor.Bucket().Get(typeKey) != nil {
205+
return id, id.Decode(mainObj)
206+
}
207+
}
208+
209+
return id, nil
210+
}

pkg/local_object_storage/metabase/revive_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,15 @@ func TestDB_ReviveObject(t *testing.T) {
4141
res, err := db.ReviveObject(object.AddressOf(raw))
4242
require.NoError(t, err)
4343
require.Equal(t, meta.ReviveStatusGraveyard, res.StatusType())
44+
require.NotNil(t, res.TombstoneAddress())
4445

4546
exists, err = metaExists(db, object.AddressOf(raw))
4647
require.NoError(t, err)
4748
require.True(t, exists)
49+
50+
exists, err = metaExists(db, tombstoneID)
51+
require.NoError(t, err)
52+
require.False(t, exists)
4853
})
4954

5055
t.Run("from GC", func(t *testing.T) {

pkg/local_object_storage/shard/put_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,47 @@ func TestDB_Put_Tombstone(t *testing.T) {
224224

225225
tombAddr := oid.NewAddress(tomb.GetContainerID(), tomb.GetID())
226226

227+
t.Run("revive after tombstone", func(t *testing.T) {
228+
sh, fs := newShardWithFSTree(t)
229+
230+
require.NoError(t, sh.Put(&obj, nil))
231+
require.NoError(t, sh.Put(&tomb, nil))
232+
233+
exist, err := sh.Exists(objAddr, false)
234+
require.Error(t, err, apistatus.ErrObjectAlreadyRemoved)
235+
require.False(t, exist)
236+
237+
_, err = sh.Get(objAddr, false)
238+
require.ErrorIs(t, err, apistatus.ErrObjectAlreadyRemoved)
239+
240+
rs, err := sh.ReviveObject(objAddr)
241+
require.NoError(t, err)
242+
require.Equal(t, meta.ReviveStatusGarbage, rs.StatusType())
243+
require.Equal(t, tombAddr, rs.TombstoneAddress())
244+
245+
exist, err = sh.Exists(objAddr, false)
246+
require.NoError(t, err)
247+
require.True(t, exist)
248+
249+
got, err := sh.Get(objAddr, false)
250+
require.NoError(t, err)
251+
require.Equal(t, &obj, got)
252+
253+
exist, err = sh.Exists(tombAddr, false)
254+
require.NoError(t, err)
255+
require.False(t, exist)
256+
257+
_, err = sh.Get(tombAddr, false)
258+
require.ErrorIs(t, err, apistatus.ErrObjectNotFound)
259+
260+
exist, err = fs.Exists(tombAddr)
261+
require.NoError(t, err)
262+
require.False(t, exist)
263+
264+
_, err = fs.Get(tombAddr)
265+
require.ErrorIs(t, err, apistatus.ErrObjectNotFound)
266+
})
267+
227268
for _, tc := range []struct {
228269
name string
229270
preset func(*testing.T, *shard.Shard)

pkg/local_object_storage/shard/revive.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package shard
33
import (
44
meta "github.com/nspcc-dev/neofs-node/pkg/local_object_storage/metabase"
55
oid "github.com/nspcc-dev/neofs-sdk-go/object/id"
6+
"go.uber.org/zap"
67
)
78

89
// ReviveObject try to revive object in Shard, by remove records from graveyard and garbage.
@@ -17,5 +18,16 @@ func (s *Shard) ReviveObject(addr oid.Address) (meta.ReviveStatus, error) {
1718
} else if s.GetMode().NoMetabase() {
1819
return meta.ReviveStatus{}, ErrDegradedMode
1920
}
20-
return s.metaBase.ReviveObject(addr)
21+
22+
st, err := s.metaBase.ReviveObject(addr)
23+
if err == nil {
24+
if ts := st.TombstoneAddress(); !ts.Object().IsZero() {
25+
if delErr := s.deleteObjs([]oid.Address{ts}); delErr != nil {
26+
s.log.Debug("tombstone delete after revive failed", zap.Stringer("tombstone", ts), zap.Error(delErr))
27+
} else {
28+
s.log.Debug("tombstone deleted after revive", zap.Stringer("tombstone", ts))
29+
}
30+
}
31+
}
32+
return st, err
2133
}

0 commit comments

Comments
 (0)