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

Don't Sync() on every upload #67

Open
buchgr opened this issue Jan 12, 2019 · 0 comments
Open

Don't Sync() on every upload #67

buchgr opened this issue Jan 12, 2019 · 0 comments

Comments

@buchgr
Copy link
Owner

buchgr commented Jan 12, 2019 •
edited
Loading

We found that on some hardware this absolutely kills the performance of the system e.g. a Mac Pro became completely unresponsive. In general, there is no need for calling Sync() on every write as in the worst case it's ok to lose data. We should only ensure that on loading the entries from disk that there is data integrity.
@buchgr buchgr added this to the 1.0 milestone Jan 12, 2019
mostynb added a commit to mostynb/bazel-remote that referenced this issue Dec 19, 2019
@mostynb
don't os.File.Sync() on mac CAS items
b8baad2 
os.File.Sync() was inneffective on mac in go prior to 1.12. In 1.12 onwards
it is effective, but slow: golang/go#26650

This change disables Sync for CAS uploads on mac, because we can (potentially)
verify this later. Let's see if this helps performance on mac: buchgr#67 (while
being less risky than buchgr#68).
@mostynb mostynb mentioned this issue Dec 19, 2019
Closed
mostynb added a commit to mostynb/bazel-remote that referenced this issue Dec 30, 2019
@mostynb
don't os.File.Sync() on mac CAS items
721c593 
os.File.Sync() was inneffective on mac in go prior to 1.12. In 1.12 onwards
it is effective, but slow: golang/go#26650

This change disables Sync for CAS uploads on mac, because we can (potentially)
verify this later. Let's see if this helps performance on mac: buchgr#67 (while
being less risky than buchgr#68).
mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 1, 2020
@mostynb
don't os.File.Sync() on mac CAS items
12603b2 
os.File.Sync() was inneffective on mac in go prior to 1.12. In 1.12 onwards
it is effective, but slow: golang/go#26650

This change disables Sync for CAS uploads on mac, because we can (potentially)
verify this later. Let's see if this helps performance on mac: buchgr#67 (while
being less risky than buchgr#68).
mostynb added a commit to mostynb/bazel-remote that referenced this issue Jan 28, 2020
@mostynb
disk cache: avoid panics when walking directories
7760cf0 
This pattern is used in the filepath.Walk godocs.

Discovered while investigating buchgr#67, committing separately so it doesn't
get lost in a larger PR.
@mostynb mostynb mentioned this issue Jan 28, 2020
Merged
mostynb added a commit that referenced this issue Jan 28, 2020
@mostynb
disk cache: avoid panics when walking directories
06ee5c9 
This pattern is used in the filepath.Walk godocs.

Discovered while investigating #67, committing separately so it doesn't
get lost in a larger PR.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 10, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
ec746f2 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
@mostynb mostynb mentioned this issue Feb 10, 2020
Open
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 10, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
62a8231 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 10, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
9c50801 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 10, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
bf25077 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 11, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
8cf9c95 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 14, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
6c1660d 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 14, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
061cfa2 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 18, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
314b7c2 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 26, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
594719e 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Feb 26, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
1b97393 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Mar 4, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
71b4293 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Mar 8, 2020
@mostynb
disk cache: store a data integrity header for non-CAS blobs
dd925de 
The header is made up of three fields:
1) Little-endian int32 (4 bytes) representing the REAPIv2
   DigestFunction.
2) Little-endian int64 (8 bytes) representing the number
   of bytes in the blob.
3) The hash bytes from the digest, length determined by
   the particular DigestFunction.
   (32 for SHA256. 20 for SHA1, 16 for MD5).

Note that we currently only support SHA256, however.

This header is simple to parse, and does not require buffering the
entire blob in memory if you just want the data.

To distinguish blobs with and without this header, we use new
directories for the affected blobs: ac.v2/ instead of ac/ and
similarly for raw/.

We do not use this header to actually verify data yet, and we
still os.File.Sync() after file writes (buchgr#67).

This also includes a slightly refactored version of PR buchgr#123
(load the items from disk concurrently) by @bdittmer.
@mostynb mostynb mentioned this issue Aug 24, 2020
Merged
@mostynb mostynb removed this from the 1.0 milestone Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@buchgr @mostynb