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

refactor: Simplify how list splits are tracked. #5920

Merged
merged 8 commits into from
Jul 24, 2020

Conversation

martinmr
Copy link
Contributor

@martinmr martinmr commented Jul 9, 2020

Currently the code keeps tracks of the split startUids and updates them
as the posting lists are split or empty parts are removed. Instead,
update the list of startUids at the end of each step based on the
contents of the map of startUids to posting list. This removes the need
of keeping track of the same piece of information in two different ways.

Also, fix a bug where the empty parts were removed from the list of
splits but not from the map. This caused empty posting lists to be
written to disk although it didn't affect normal execution because those
parts were not accessible from the main posting list.


This change is Reviewable

Docs Preview: Dgraph Preview

Currently the code keeps tracks of the split startUids and updates them
as the posting lists are split or empty parts are removed. Instead,
update the list of startUids at the end of each step based on the
contents of the map of startUids to posting list. This removes the need
of keeping track of the same piece of information in two different ways.

Also, fix a bug where the empty parts were removed from the list of
splits but not from the map. This caused empty posting lists to be
written to disk although it didn't affect normal execution because those
parts were not accessible from the main posting list.
@martinmr martinmr requested a review from manishrjain as a code owner July 9, 2020 22:43
@martinmr martinmr requested a review from a team July 9, 2020 22:43
@martinmr martinmr changed the title Simplify how list splits are tracked. refactor: Simplify how list splits are tracked. Jul 9, 2020
@martinmr martinmr marked this pull request as draft July 10, 2020 21:54
@martinmr martinmr marked this pull request as ready for review July 21, 2020 15:12
Copy link
Contributor

@ashish-goswami ashish-goswami left a comment

Choose a reason for hiding this comment

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

:lgtm:

Please make sure CI is green before merging.

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)

Copy link
Contributor

@parasssh parasssh left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)

@martinmr martinmr merged commit c51d007 into master Jul 24, 2020
@martinmr martinmr deleted the martinmr/split-refactor branch July 24, 2020 20:19
martinmr added a commit that referenced this pull request Jul 24, 2020
Currently the code keeps tracks of the split startUids and updates them
as the posting lists are split or empty parts are removed. Instead,
update the list of startUids at the end of each step based on the
contents of the map of startUids to posting list. This removes the need
of keeping track of the same piece of information in two different ways.

Also, fix a bug where the empty parts were removed from the list of
splits but not from the map. This caused empty posting lists to be
written to disk although it didn't affect normal execution because those
parts were not accessible from the main posting list.

(cherry picked from commit c51d007)
martinmr added a commit that referenced this pull request Jul 24, 2020
Currently the code keeps tracks of the split startUids and updates them
as the posting lists are split or empty parts are removed. Instead,
update the list of startUids at the end of each step based on the
contents of the map of startUids to posting list. This removes the need
of keeping track of the same piece of information in two different ways.

Also, fix a bug where the empty parts were removed from the list of
splits but not from the map. This caused empty posting lists to be
written to disk although it didn't affect normal execution because those
parts were not accessible from the main posting list.

(cherry picked from commit c51d007)
martinmr added a commit that referenced this pull request Jul 24, 2020
Currently the code keeps tracks of the split startUids and updates them
as the posting lists are split or empty parts are removed. Instead,
update the list of startUids at the end of each step based on the
contents of the map of startUids to posting list. This removes the need
of keeping track of the same piece of information in two different ways.

Also, fix a bug where the empty parts were removed from the list of
splits but not from the map. This caused empty posting lists to be
written to disk although it didn't affect normal execution because those
parts were not accessible from the main posting list.

(cherry picked from commit c51d007)
parasssh pushed a commit that referenced this pull request Aug 28, 2020
Currently the code keeps tracks of the split startUids and updates them
as the posting lists are split or empty parts are removed. Instead,
update the list of startUids at the end of each step based on the
contents of the map of startUids to posting list. This removes the need
of keeping track of the same piece of information in two different ways.

Also, fix a bug where the empty parts were removed from the list of
splits but not from the map. This caused empty posting lists to be
written to disk although it didn't affect normal execution because those
parts were not accessible from the main posting list.

(cherry picked from commit c51d007)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants