Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework Timestamp attribute of block objects in NeoFS block storage #3654

Open
AnnaShaleva opened this issue Nov 1, 2024 · 6 comments
Open
Labels
enhancement Improving existing functionality I3 Minimal impact S4 Routine U4 Nothing urgent
Milestone

Comments

@AnnaShaleva
Copy link
Member

AnnaShaleva commented Nov 1, 2024

Is your feature request related to a problem? Please describe.

Block objects stored in the NeoFS block storage have Timestamp attribute which is currently the block timestamp in milliseconds. This behaviour results in the following:

$ ./bin/neofs-cli object head -r minerva.nspcc.ru:8080 --cid VCVr6RKg1hfJR7v36xvjbaXTncj1waPi94u9EFyEttW --oid 4vMdkXbQEgnSSiEi7xsywwoqpbdS6mgqPFSSouyqXAEj --ttl 1
ID: 4vMdkXbQEgnSSiEi7xsywwoqpbdS6mgqPFSSouyqXAEj
CID: VCVr6RKg1hfJR7v36xvjbaXTncj1waPi94u9EFyEttW
Owner: NhNT6EFqrDu7hAsv2V1J3B7J1DKLvkNGRg
CreatedAt: 1
Size: 697
HomoHash: 13e0148adc711a768d37a14434fac38f48877691f17e4b312b107a4cdbce3427322524d59acd97bcff1a9f554db3348277cff084aa6ff9ab573787bf605b61c2
Checksum: 6ac6d21575d25263b31194f5f81498b9971d1c21367849fcb325d792c8f0452e
Type: REGULAR
Attributes:
  Block=1976
  Primary=2
  Hash=7c27f25d29092d63ad0ec8b0aa284903be12610ac581a90c6ddceb47c514d5c9
  PrevHash=d2d12394af799d97512d715375a72ddea29fb64824797d1e52a04bc672a4e31b
  Timestamp=1628131414239 (53563-06-07 17:10:39 +0300 MSK)
ID signature:
  public key: 02d89e1e06504a2ec3410a9ea801acfa3e45ea5d9ad3fa56ea179638a6dd72b7d2
  signature: 51d71990d5cce28e10d07a2e7c6038da1158e26e7b7d37b6ae78bd5094b88ec1462d17be6c2adb18141f9330b4a6102063ffb0e859fe6c8f9a5f410517959344

Here we see this strange Timestamp=1628131414239 (53563-06-07 17:10:39 +0300 MSK) record because NeoFS API treats Timestamp attribute as Unix time of object creation: https://github.com/nspcc-dev/neofs-api/blob/d7c033317a66aff467728f4ca5a86879edb66e54/object/types.proto#L162.

Describe the solution you'd like

Rename Timestamp to TimestampMilli and don't set Timestamp attribute at all.

Describe alternatives you've considered

Convert Timestamp attribute from milliseconds-precision to Unix timestamp. But the meaning of this field is still not "time of object creation" anyway. It's the time of block creation.

Additional context

Please, vote:
👍 for renaming
👎 for conversion to Unix timestamp

It's a breaking change, hence it must be handled with care because we already have existing block objects uploaded using the old format.

@AnnaShaleva AnnaShaleva added I3 Minimal impact U4 Nothing urgent enhancement Improving existing functionality S4 Routine labels Nov 1, 2024
@AnnaShaleva AnnaShaleva added this to the v0.107.0 milestone Nov 1, 2024
@AliceInHunterland
Copy link
Contributor

Rename Timestamp to TimestampMilli, remove Timestamp attribute at all.

We will have more duplicates if the Timestamp won't block creation time. (we won't be afraid only after #3652 )

@AnnaShaleva
Copy link
Member Author

We will have more duplicates if the Timestamp won't block creation time

That's why I propose to explicitly remove Timestamp attribute.

we won't be afraid only after #3652

Exactly.

@AliceInHunterland
Copy link
Contributor

Why TimestampMilli? Maybe BlockTimestamp or something block related?

@AnnaShaleva
Copy link
Member Author

AnnaShaleva commented Nov 1, 2024

Maybe BlockTimestamp or something block related?

All block object attributes are block-related, hence I consider any Block* prefix redundant.

@AliceInHunterland
Copy link
Contributor

All block object attributes are block-related, hence I consider any Block* prefix redundant.

If we rename it, we will have a Timestamp attribute - the default one from NeoFS and our block-related timestamp. In the case Timestamp + TimestampMilli it's not obvious which one block-related.
Also instead of Index of the block, we have Block as a block-related attribute to identify it, so the Block* prefix could be helpful.

@AnnaShaleva
Copy link
Member Author

the default one from NeoFS

This default is not written in stones, it may easily be removed, it's a matter of an additional method in SDK.

Also instead of Index of the block, we have Block as a block-related attribute to identify it,

Block index is a special one, it combines the block object identifier with the index. We anyway have to include block object identifier into the list of attributes, and it's reused for index as well in order not to waste space for a separate Index attribute.

so the Block* prefix could be helpful.

So I'm still not convinced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality I3 Minimal impact S4 Routine U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

2 participants