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

[fix] #2501: Fixes the order of assets in queries with pagination #2515

Closed

Conversation

pesterev
Copy link
Contributor

@pesterev pesterev commented Jul 21, 2022

Signed-off-by: Vladimir Pesterev pesterev@pm.me

Description of the Change

Replaces the lexicographical order of assets with a timestamp-based order in AssetsMap.

For example:

Before: 1 (1658432537), 10 (1658432546), 2 (1658432538), 3 (1658432539), 4 (1658432540), 5 (1658432541), 6 (1658432542), 7 (1658432543), 8 (1658432544), 9 (1658432545).

Now: 1 (1658432537), 2 (1658432538), 3 (1658432539), 4 (1658432540), 5 (1658432541), 6 (1658432542), 7 (1658432543), 8 (1658432544), 9 (1658432545), 10 (1658432546).

Where N is Id and T is an insertion timestamp in N (T) format.

Issue

Resolves #2501

Benefits

Order now depends on insertion timestamp.

Possible Drawbacks

The queries that retrieve an asset will execute a bit slower because $O(\log n)$ (current implementation) and $O (\log n + \log n)$ (twice b-tree lookup) (impl in this PR).

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Jul 21, 2022
@pesterev pesterev marked this pull request as ready for review July 21, 2022 19:48
data_model/src/asset.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@mversic mversic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't the problem that we have inconsistent ordering, i.e. every query returns assets in a different order? Or is the problem that assets are sorted alphabetically and not chronologically?

@Erigara Erigara self-assigned this Jul 22, 2022
@mversic mversic self-requested a review July 22, 2022 10:25
@Erigara Erigara removed their assignment Jul 26, 2022
…th pagination

Signed-off-by: Vladimir Pesterev <pesterev@pm.me>
@pesterev pesterev marked this pull request as draft July 28, 2022 09:01
@pesterev
Copy link
Contributor Author

Convert this to a draft because the current decision affects memory related optimizations.

@mversic
Copy link
Contributor

mversic commented Aug 4, 2022

@pesterev I believe we can close this in favor of #2575. Please close it if so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants