Fix/Allow reading expired locked object #56

carpawell · 2023-02-15T16:27:32Z

carpawell commented

2023-02-15 16:27:32 +00:00

(Migrated from github.com)

Related to https://github.com/TrueCloudLab/frostfs-api/issues/2.

acid-ant (Migrated from github.com) reviewed 2023-02-16 05:33:17 +00:00

fyrchik (Migrated from github.com) reviewed 2023-02-16 06:05:54 +00:00

pkg/local_object_storage/metabase/exists.go

													
				@ -103,0 +104,4 @@

					if objectLocked(tx, addr.Container(), addr.Object()) {

						return 0

					}

fyrchik (Migrated from github.com) commented

2023-02-16 06:05:49 +00:00

Can we have an explicit unit-test for this behaviour?

carpawell (Migrated from github.com) reviewed 2023-02-17 14:14:37 +00:00

pkg/local_object_storage/metabase/exists.go

													
				@ -103,0 +104,4 @@

					if objectLocked(tx, addr.Container(), addr.Object()) {

						return 0

					}

carpawell (Migrated from github.com) commented

2023-02-17 14:14:36 +00:00

sure, added

acid-ant (Migrated from github.com) reviewed 2023-02-17 14:24:38 +00:00

fyrchik (Migrated from github.com) reviewed 2023-02-20 10:47:36 +00:00

pkg/local_object_storage/metabase/lock_test.go

													
				@ -172,0 +183,4 @@

					require.ErrorIs(t, err, meta.ErrObjectIsExpired)

					// lock the obj

					require.NoError(t, db.Lock(addr.Container(), oidtest.ID(), []oid.ID{addr.Object()}))

fyrchik (Migrated from github.com) commented

2023-02-20 10:47:33 +00:00

Can we lock an already expired object or is it only for tests here?

Can we _lock_ an already expired object or is it only for tests here?

carpawell (Migrated from github.com) reviewed 2023-02-20 10:53:25 +00:00

pkg/local_object_storage/metabase/lock_test.go

													
				@ -172,0 +183,4 @@

					require.ErrorIs(t, err, meta.ErrObjectIsExpired)

					// lock the obj

					require.NoError(t, db.Lock(addr.Container(), oidtest.ID(), []oid.ID{addr.Object()}))

carpawell (Migrated from github.com) commented

2023-02-20 10:53:25 +00:00

Can we lock an already expired object

yes, and i do not see any problems with that, you? in practice i think that is almost impossible if GC is configured to be relatively fast

> Can we lock an already expired object yes, and i do not see any problems with that, you? in practice i think that is almost impossible if GC is configured to be relatively fast

fyrchik (Migrated from github.com) reviewed 2023-02-20 12:51:53 +00:00

pkg/local_object_storage/metabase/lock_test.go

													
				@ -172,0 +183,4 @@

					require.ErrorIs(t, err, meta.ErrObjectIsExpired)

					// lock the obj

					require.NoError(t, db.Lock(addr.Container(), oidtest.ID(), []oid.ID{addr.Object()}))

fyrchik (Migrated from github.com) commented

2023-02-20 12:51:52 +00:00

Unexpired object should not exits, LOCKing it postfactum seems wrong.

Unexpired object should not exits, `LOCK`ing it postfactum seems wrong.

fyrchik (Migrated from github.com) reviewed 2023-02-20 12:52:04 +00:00

pkg/local_object_storage/metabase/lock_test.go

													
				@ -172,0 +183,4 @@

					require.ErrorIs(t, err, meta.ErrObjectIsExpired)

					// lock the obj

					require.NoError(t, db.Lock(addr.Container(), oidtest.ID(), []oid.ID{addr.Object()}))