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

Check block hash before writing #19

Merged
merged 3 commits into from
Jan 14, 2019
Merged

Conversation

muhamadazmy
Copy link
Member

Also make sure to sync before using the file to avoid
corruption on crash

Also make sure to sync before using the file to avoid
corruption on crash
@ghost ghost assigned muhamadazmy Jan 14, 2019
@ghost ghost added the state_inprogress label Jan 14, 2019
@maxux
Copy link
Collaborator

maxux commented Jan 14, 2019

What's the performance impact ?

@codecov-io
Copy link

codecov-io commented Jan 14, 2019

Codecov Report

Merging #19 into development will decrease coverage by 0.23%.
The diff coverage is 30.76%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development      #19      +/-   ##
===============================================
- Coverage        34.48%   34.24%   -0.24%     
===============================================
  Files               10       10              
  Lines              464      476      +12     
===============================================
+ Hits               160      163       +3     
- Misses             279      284       +5     
- Partials            25       29       +4
Impacted Files Coverage Δ
rofs/cache.go 0% <0%> (ø) ⬆️
rofs/downloader.go 75.34% <33.33%> (-8.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0321ab...db71955. Read the comment docs.

@muhamadazmy
Copy link
Member Author

What's the performance impact ?

It's really hard to tell. The performance is impacted by lots of other factors like internet connection speed. I can't do any repetitive measurements.

@muhamadazmy
Copy link
Member Author

I will benchmark blak2b separately without the rest

@muhamadazmy
Copy link
Member Author

I think the overhead is pretty low

image

return nil, err
}

hasher, err := blake2b.New(16, nil)
Copy link

Choose a reason for hiding this comment

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

Do you need a new hasher everytime ? can't we have a single hasher and just reset it? Would avoid allocation

Copy link

@zaibon zaibon left a comment

Choose a reason for hiding this comment

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

LGTM

@muhamadazmy muhamadazmy merged commit 19b8df0 into development Jan 14, 2019
@ghost ghost removed the state_inprogress label Jan 14, 2019
@muhamadazmy muhamadazmy deleted the development-block-check branch January 14, 2019 10:57
muhamadazmy pushed a commit that referenced this pull request Jan 16, 2019
* Block hash checking

Also make sure to sync before using the file to avoid
corruption on crash

* Fix tests

* Benchmarking blak2b/128
muhamadazmy pushed a commit that referenced this pull request Sep 18, 2020
* Check block hash before writing (#19)

* Block hash checking

Also make sure to sync before using the file to avoid
corruption on crash

* Fix tests

* Benchmarking blak2b/128

* Allow 0-fs to start as a daemon (#21)

* Allow 0-fs to start as a daemon

- also make stderr the default log output stream
- add flag to enable daemon mode and pid of the daemon

* vendore new dependencies

* fix a regression bug

This line was removed on a previous PR.

* Make sure that the rw layer has same perm as ro

This is to make sure that the final merge mount point
has same permissions as the flist

* Support ReadOnly flag during mounting (#24)

* WIP: support ro flag

Add code to support mounting a ro directly
in that case, we don't do overlay

* Make sure if wait fails we cleanly unmount

* Original mode works fine

* Add ro flag to cmd line interface

* use go1.13 for tests

* fix connection management in redis pool (#27)

* fix connection management in redis pool

The pool configuration requires to have the MaxIdle connection to a
number bigger then 1 to allow to reuse connection.
Without it a new connection is always created.

This commit allow to have a total of 10 connection in the pool and keep
2 of them open.

* Increase redis pool settings for small improvements

Co-authored-by: Maxime Daniel <root@maxux.net>

* Go mod (#30)

* WIP: support uid/gid values in acl

* use simple int32 with default -1 instead

* use int64

* Support uid/gid numbers

* WIP: fix uid/gid

* Finally found the issue with uid/gid

* remove log line

* remove replace directive in go.mod

* move from travis-ci to GithubAction

* add more static checks during CI

* address linting and statickcheck warnings

* update badge on readme

* Parent process should only exit successfully after mount is started (#36)

* Parent process should only exit successfully is mount is okay

* fix sleep

* Dev/go/upgrade version (#37)

* Upgrade go version in workflows

- Also run `go mod tidy`

* make vet happy

* fix make file

Co-authored-by: Christophe de Carvalho <zaibon@users.noreply.github.com>
Co-authored-by: Maxime Daniel <root@maxux.net>
Co-authored-by: Christophe de Carvalho <christophe.dcpm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants