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.

Sign up for GitHub

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

Jump to bottom

refactor: Store only key in DAG instead of dockey path #1245

Merged
merged 4 commits into from
Mar 29, 2023
Merged

refactor: Store only key in DAG instead of dockey path #1245

merged 4 commits into from
Mar 29, 2023

Conversation

islamaliev
Copy link
Contributor

@islamaliev islamaliev commented Mar 28, 2023 •
edited
Loading

Relevant issue(s)

Resolves #1242

Description

This PR makes deltas to be stored with just dockey hash instead of the whole dockey path.
It means that this PR introduces BREAKING CHANGES and makes previous CIDs invalid.

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Integration tests

Specify the platform(s) on which this was tested:

  • MacOS

@islamaliev islamaliev added feature New feature or request area/datastore Related to the datastore / storage engine system action/no-benchmark Skips the action that runs the benchmark. labels Mar 28, 2023
@islamaliev islamaliev added this to the DefraDB v0.5 milestone Mar 28, 2023
@islamaliev islamaliev self-assigned this Mar 28, 2023
@codecov
Copy link

codecov bot commented Mar 28, 2023 •
edited
Loading

Codecov Report

Merging #1245 (0195159) into develop (c9ef176) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1245      +/-   ##
===========================================
- Coverage    70.70%   70.69%   -0.02%     
===========================================
  Files          182      182              
  Lines        17225    17221       -4     
===========================================
- Hits         12179    12174       -5     
- Misses        4117     4118       +1     
  Partials       929      929
Impacted Files Coverage Δ
core/crdt/composite.go 76.00% <100.00%> (ø)
core/crdt/lwwreg.go 76.66% <100.00%> (ø)
planner/commit.go 81.62% <100.00%> (+1.19%) ⬆️

... and 6 files with indirect coverage changes

shahzadlone
shahzadlone approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

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

Sorry for having to change cids again 😭. LGTM

fredcarle
fredcarle approved these changes Mar 28, 2023
View reviewed changes
Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM :)

AndrewSisley
AndrewSisley approved these changes Mar 28, 2023
View reviewed changes
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@islamaliev islamaliev merged commit f767394 into develop Mar 29, 2023
@islamaliev islamaliev deleted the islam/feat/I1242-store-only-key-in-dag branch March 29, 2023 05:29
@fredcarle fredcarle changed the title feat: Store only key in DAG instead of dockey path refactor: Store only key in DAG instead of dockey path Apr 3, 2023
@fredcarle fredcarle added refactor This issue specific to or requires *notable* refactoring of existing codebases and components and removed feature New feature or request labels Apr 3, 2023
shahzadlone pushed a commit that referenced this pull request Apr 13, 2023
@islamaliev @shahzadlone
feat: Store only key in DAG instead of dockey path (#1245)
2e73dc0 
Store only dockey hash instead of whole path in DAG

Adjust CIDs in tests
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
@islamaliev
feat: Store only key in DAG instead of dockey path (sourcenetwork#1245)
633f0e6 
Store only dockey hash instead of whole path in DAG

Adjust CIDs in tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fredcarle fredcarle fredcarle approved these changes

@shahzadlone shahzadlone shahzadlone approved these changes

@AndrewSisley AndrewSisley AndrewSisley approved these changes

@jsimnz jsimnz Awaiting requested review from jsimnz

Assignees

@islamaliev islamaliev

Labels
action/no-benchmark Skips the action that runs the benchmark. area/datastore Related to the datastore / storage engine system refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Milestone
DefraDB v0.5
Development

Successfully merging this pull request may close these issues.

Store in DAG just a key instead of the whole dockey path
4 participants
@islamaliev @fredcarle @shahzadlone @AndrewSisley