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

Support Acquire-By-Hash for index files #551

Closed
wants to merge 2 commits into from
Closed

Support Acquire-By-Hash for index files #551

wants to merge 2 commits into from

Conversation

neolynx
Copy link
Member

@neolynx neolynx commented Apr 15, 2017

The added "aptly publish repo" option "-access-by-hash" publishes
the index files (Packages*, Sources*) also as hardlinked hashes.
Example:
/dists/yakkety/main/binary-amd64/by-hash/SHA512/31833ec39acc...
The Release files indicate this with the option "Acquire-By-Hash: yes"

This is used by apt >= 1.2.0 and prevents the "Hash sum mismatch" race
condition between a server side "aptly publish repo" and "apt-get update"
on a client.
See: http://www.chiark.greenend.org.uk/~cjwatson/blog/no-more-hash-sum-mismatch-errors.html

This implementation uses symlinks in the by-hash/*/ directory for keeping
only two versions of the index files and deleting older files
automatically.

Note: this only works with aptly.FileSystemPublishedStorage

Closes: #536

Signed-off-by: André Roth neolynx@gmail.com

@smira
Copy link
Contributor

smira commented Apr 17, 2017

I wonder if we should expose "symlinking" for S3/Swift supported publishing destinations (it could be as simple as PUT Copy)?

@neolynx
Copy link
Member Author

neolynx commented Apr 18, 2017

S3 does not support symlinks, so we could put the name of the hash into the file instead of creating a symlink. We could also do this for the local file system, or keep the symlinks there.

What would you prefer?

@smira
Copy link
Contributor

smira commented Apr 18, 2017

I think we should try to avoid going with special-case depending on PublishedStorage. I would rather add new method Symlink to the interface of PublishedStorage, and implement it accordingly for each storage type - local files published storage can really create symlink, while S3/Swift can do PUT copy to emulate symlinks.

@neolynx
Copy link
Member Author

neolynx commented Apr 19, 2017

Agree, I suggest to add the following to the PublishedStorage interface:

  • PutSymlink(src, dst) creates symlink or file containing link dst
  • ReadLink(path) returns readlink or content of file, nil if file not found
  • PutHardlink(src, dst) created hardlink or file copy
  • RemoveFile(path)
  • in general: all non-existing directories will be created automatically

Does this sound reasonable?

@smira
Copy link
Contributor

smira commented Apr 21, 2017

Oh, I didn't know we can put filename as fallback option. I think this would work the best and that even doesn't require any changes to PublishedStorage interface?

I'm trying to keep it simple if possible.

@neolynx
Copy link
Member Author

neolynx commented Apr 23, 2017

The fallback is only for the houskeeping. The interface has to be extended to support creating hardlinks (or a copy of the file when hardlinks are not available) with an arbitrary (hash) name. I agree on keeping the interface simple, but I think the acquire-by-hash support needs these 4 extensions in the interface.
Alternatively we could modify the interface to receive the file object, and do the acquire-by-hash handling in the storage.

What do you prefer?

@neolynx
Copy link
Member Author

neolynx commented May 21, 2017

Hi,
I've updated the PublishedStorage interface with the following functions:

  • // SymLink creates a symbolic link, which can be read with ReadLink
  • SymLink(src string, dst string) error
  • // HardLink creates a hardlink of a file
  • HardLink(src string, dst string) error
  • // FileExists returns true if path exists
  • FileExists(path string) bool
  • // ReadLink returns the symbolic link pointed to by path
  • ReadLink(path string) (string, error)

This allows the Acquire-By-Hash to be implemented by other the storage backends s3 and swift. If hardlinks are not supported, the file can simply be copied, if symlinks are not supported, the destination can be written to a file, which can be read and returned by ReadLink. FileExists is for convenience.

If this is OK with you, I'll implement the s3 support, maybe someone else can implement these functions for swift.

@smira
Copy link
Contributor

smira commented May 22, 2017

Thanks, @neolynx. Plan sounds good to me. I didn't quite get to the point where I can do through review, but I keep this PR on my radar.

@smira smira added 1.2.0 and removed 1.1.0-Maybe labels Jul 5, 2017
The added "aptly publish repo" option "-access-by-hash" publishes
the index files (Packages*, Sources*) also as hardlinked hashes.
Example:
 /dists/yakkety/main/binary-amd64/by-hash/SHA512/31833ec39acc...
The Release files indicate this with the option "Acquire-By-Hash: yes"

This is used by apt >= 1.2.0 and prevents the "Hash sum mismatch" race
condition between a server side "aptly publish repo" and "apt-get update"
on a client.
See: http://www.chiark.greenend.org.uk/~cjwatson/blog/no-more-hash-sum-mismatch-errors.html

This implementation uses symlinks in the by-hash/*/ directory for keeping
only two versions of the index files and deleting older files
automatically.

Note: this only works with aptly.FileSystemPublishedStorage

Closes: #536

Signed-off-by: André Roth <neolynx@gmail.com>
Signed-off-by: André Roth <neolynx@gmail.com>
@smira
Copy link
Contributor

smira commented Nov 20, 2017

Closing in favor of #664

@smira smira closed this Nov 20, 2017
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.

2 participants