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

[Release candidate] v10 prerelease #233

Closed
wants to merge 111 commits into from
Closed

[Release candidate] v10 prerelease #233

wants to merge 111 commits into from

Conversation

mafintosh
Copy link
Contributor

@mafintosh mafintosh commented Feb 6, 2019

@andrewosh and I (but mostly @andrewosh) have been busy getting Hyperdrive v10 ready to ship. It's a major change that moves the internal filesystem structure to hypertrie, for massive perf boosts.

The is the first "official" step towards multiwriter as the trie enables the data structures needed for that, but v10 will still be a single writer only hyperdrive, focused on getting the trie out so we can get some milage on that.

Also massively increases test coverage.

NOTE: we are still finishing up the last parts of the API

Prerelease Checklist

  • Support for writable file descriptors
  • Stateful file descriptor fuzz test(s)
  • Benchmark + test completeness with a barebones FUSE implemention (minus random-access writes)
  • Support for "live" files (writing a stat object at the beginning of the file write with a live flag)

@andrewosh andrewosh changed the title v10 prerelease [WIP] v10 prerelease Feb 6, 2019
Copy link
Contributor

@martinheidegger martinheidegger left a comment

Choose a reason for hiding this comment

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

A few comments when reading over the PR.

index.js Outdated Show resolved Hide resolved
index.js Outdated
* Initialize the db without metadata and load the content feed key from the header.
*/
if (this._db) {
if (!this.contentFeed || !this.metadataFeed) return cb(new Error('Must provide a db and both content/metadata feeds'))
Copy link
Contributor

Choose a reason for hiding this comment

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

This error will be hard to debug, that problem is in the constructor (where the db is passed), the error should be thrown there.

index.js Outdated Show resolved Hide resolved
lib/keys.js Outdated

function contentKeyPair (secretKey) {
var seed = Buffer.allocUnsafe(sodium.crypto_sign_SEEDBYTES)
var context = Buffer.from('hyperdri', 'utf8') // 8 byte context
Copy link
Contributor

Choose a reason for hiding this comment

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

The context never changes, why not put it in a constant?

lib/stat.js Outdated
this.offset = (data && data.offset) || 0
this.byteOffset = (data && data.byteOffset) || 0
this.blocks = (data && data.blocks) || 0
this.atime = data && data.atime ? getTime(data.atime) : 0 // we just set this to mtime ...
Copy link
Contributor

Choose a reason for hiding this comment

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

I know that comment was there before, but that comment makes little sense to me.

lib/content.js Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
this._openFiles[fd - 20] = null
cursor.close(cb)
}
exists (name, opts, cb) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Its deprecated in node.js; should it stay implemented?

index.js Outdated Show resolved Hide resolved
@yoshuawuyts
Copy link

@mafintosh are you planning to release a blog post when v10 is released? Even if it's short, this seems like it might be a good opportunity to do a bit of marketing, and helping others catch up on what's changed.

@mafintosh
Copy link
Contributor Author

Rebased into master 💥 🎉

@mafintosh mafintosh closed this Jul 30, 2019
@mafintosh mafintosh deleted the v10 branch July 30, 2019 14:03
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.

6 participants