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

feat(replica): run fsync on files & dir after create/remove/rename operation on files #278

Merged
merged 8 commits into from
Apr 14, 2020
Merged

feat(replica): run fsync on files & dir after create/remove/rename operation on files #278

merged 8 commits into from
Apr 14, 2020

Conversation

utkarshmani1997
Copy link
Contributor

@utkarshmani1997 utkarshmani1997 commented Mar 6, 2020 •
edited
Loading

This commit enhances the following:

  • Do fsync on files and directories after create/remove/rename operation

Note:

Signed-off-by: Utkarsh Mani Tripathi [email protected]

@utkarshmani1997 utkarshmani1997 requested a review from payes March 6, 2020 13:27
@utkarshmani1997 utkarshmani1997 self-assigned this Mar 6, 2020
@utkarshmani1997 utkarshmani1997 added this to the 1.8.0 milestone Mar 6, 2020
golangcibot
golangcibot reviewed Mar 6, 2020
View reviewed changes
@utkarshmani1997 utkarshmani1997 force-pushed the sync branch 2 times, most recently from e79d8c7 to 56b7f45 Compare March 10, 2020 07:41
@payes payes modified the milestones: 1.8.0, 1.9.0 Mar 18, 2020
@vishnuitta
Copy link
Contributor

  • Is this issue reproduced by performing node restarts and did we observed inconsistent blocks across replicas?
  • If so, does this regular sync (say 5 seconds) helps (with the current way of checking for chain and meta files during sync process)?

@utkarshmani1997 utkarshmani1997 force-pushed the sync branch 3 times, most recently from 834292f to 5fe15e3 Compare April 3, 2020 09:41
@utkarshmani1997 utkarshmani1997 changed the title feat(replica): do periodic sync on files & dir feat(replica): run fsync on files & dir after create/remove/rename operation on files Apr 3, 2020
vishnuitta
vishnuitta reviewed Apr 6, 2020
View reviewed changes
vishnuitta
vishnuitta reviewed Apr 6, 2020
View reviewed changes
vishnuitta
vishnuitta reviewed Apr 6, 2020
View reviewed changes
vishnuitta
vishnuitta reviewed Apr 6, 2020
View reviewed changes
vishnuitta
vishnuitta reviewed Apr 7, 2020
View reviewed changes
Utkarsh Mani Tripathi added 7 commits April 13, 2020 18:47
feat(replica): do periodic sync on files & dir
2d83319 
We open file in O_DIRECT mode which minimizes the cache effects but doesn't
guarantee that data is written to the disk. Periodic sync ensures that both
data and metadata is flushed to the disk at every 5 seconds. See man page of
open system call for more details. Also since fsync on fd's also doesn't
guarantee the same so fsync on directory is also required.

In this commit a goroutine is launched to perform sync on data files and
directory. Also sync and close the replicas gracefully before fataling upon
error or killing the replicas.

Signed-off-by: Utkarsh Mani Tripathi <[email protected]>
fix(golint-ci): address errCheck issues
6c93e05 
Signed-off-by: Utkarsh Mani Tripathi <[email protected]>
fix(replica): log ignored errors
60cdfdf 
Signed-off-by: Utkarsh Mani Tripathi <[email protected]>
chore(replica): remove periodic sync goroutine
f56b29e 
Signed-off-by: Utkarsh Mani Tripathi <[email protected]>
chore(replica): rm gracful shutdown related code
8b8be0d 
Signed-off-by: Utkarsh Mani Tripathi <[email protected]>
chore(replica): fsync dir after all other missing places
ab3d194 
Signed-off-by: Utkarsh Mani Tripathi <[email protected]>
fix(replica): add missing f.Close in if case
e0c9be0 
Signed-off-by: Utkarsh Mani Tripathi <[email protected]>
vishnuitta
vishnuitta reviewed Apr 13, 2020
View reviewed changes
vishnuitta
vishnuitta approved these changes Apr 13, 2020
View reviewed changes
Copy link
Contributor

@vishnuitta vishnuitta left a comment

Choose a reason for hiding this comment

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

changes are good.. one small minor comment..
in coming PR, we need to handle the comment added in this PR " // file created before this needs to be deleted in case of error"

fix(replica): replace err with closeErr
80a5637 
Signed-off-by: Utkarsh Mani Tripathi <[email protected]>
@payes payes merged commit 2b621d1 into openebs-archive:master Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@golangcibot golangcibot golangcibot left review comments

@payes payes payes approved these changes

@vishnuitta vishnuitta vishnuitta approved these changes

Assignees

@utkarshmani1997 utkarshmani1997

Labels
enhancement pr/hold-merge pr/release-note refactor
Projects
None yet
Milestone
1.9.0
Development

Successfully merging this pull request may close these issues.
4 participants
@utkarshmani1997 @vishnuitta @payes @golangcibot