Fix/Meta returns old expired objects #262

carpawell · 2023-04-17T17:25:53Z

carpawell commented

2023-04-17 17:25:53 +00:00

No description provided.

carpawell self-assigned this 2023-04-17 17:25:53 +00:00

carpawell changed title from ~~fix/Meta returns old expired objects~~ to Fix/Meta returns old expired objects

2023-04-17 17:26:01 +00:00

carpawell referenced this pull request from a commit

2023-04-17 17:32:25 +00:00

[#262] meta: Do not return old expired objects

carpawell force-pushed fix/meta-returns-old-expired-objects from ed632f904e to 536ab6ec81

2023-04-17 17:32:27 +00:00

Compare

carpawell requested review from storage-core-developers 2023-04-17 17:32:52 +00:00

carpawell requested review from storage-core-committers 2023-04-17 17:32:52 +00:00

carpawell requested review from fyrchik 2023-04-17 17:32:56 +00:00

dstepanov-yadro approved these changes 2023-04-18 05:08:42 +00:00

acid-ant approved these changes 2023-04-18 07:32:52 +00:00

fyrchik reviewed 2023-04-18 07:56:13 +00:00

pkg/local_object_storage/metabase/util.go Outdated

					
				@ -261,1 +263,4 @@

				}

				// performs an unmarshalled object validity checks.

				func unmarshalAndValidate(data []byte, currEpoch uint64) (*object.Object, error) {

fyrchik commented

2023-04-18 07:56:13 +00:00

Why have you decided to validate here instead of in final Get step, right before returning a result?

Why have you decided to validate here instead of in final `Get` step, right before returning a result?

carpawell commented

2023-04-18 14:59:09 +00:00

can you, please, add details a little? validate what?

it is just best we can do if we have already unmarshaled an obj, get has multiple returns so i just moved unmarshaling to that func. do you mean do validation via defer?

can you, please, add details a little? validate what? it is just best we can do if we have already unmarshaled an obj, `get` has multiple `return`s so i just moved unmarshaling to that func. do you mean do validation via `defer`?

fyrchik commented

2023-04-19 09:28:49 +00:00

I mean checking this not inside the transaction, but outside.
.Attributes() allocates, iteration is not as fast as it could be and write tx can only execute in 1 thread.

I mean checking this not inside the transaction, but outside. `.Attributes()` allocates, iteration is not as fast as it could be and write tx can only execute in 1 thread.

fyrchik commented

2023-04-19 10:52:18 +00:00

get is used in Delete, for example, do we have a test for deleting an expired object?

`get` is used in `Delete`, for example, do we have a test for deleting an expired object?

carpawell commented

2023-04-19 13:08:52 +00:00

I mean checking this not inside the transaction, but outside.

oh, i see, did not think about that and forget at all that it executes inside a TX, fixed

do we have a test for deleting an expired object?

yes, we have a couple

> I mean checking this not inside the transaction, but outside. oh, i see, did not think about that and forget at all that it executes inside a TX, fixed > do we have a test for deleting an expired object? yes, we have a couple