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

Add virtual Truncate method to Env #3779

Closed
wants to merge 2 commits into from

Conversation

nvanbenschoten
Copy link
Contributor

This change adds a virtual Truncate method to Env, which truncates
the named file to the specified size. At the moment, this is only
supported for MockEnv, but other Env's could be extended to override
the method too. This is the same approach that methods like LinkFile and
AreSameFile have taken.

This is useful for any user of the in-memory Env. The implementation's
header is not exported, so before this change, it was impossible to
access it's already existing Truncate method.

This change adds a virtual `Truncate` method to `Env`, which truncates
the named file to the specified size. At the moment, this is only
supported for `MockEnv`, but other `Env's` could be extended to override
the method too. This is the same approach that methods like `LinkFile` and
`AreSameFile` have taken.

This is useful for any user of the in-memory `Env`. The implementation's
header is not exported, so before this change, it was impossible to
access it's already existing `Truncate` method.
Copy link
Contributor

@ajkr ajkr left a comment

Choose a reason for hiding this comment

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

makes sense

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@nvanbenschoten has updated the pull request. View: changes, changes since last import

@ajkr
Copy link
Contributor

ajkr commented Apr 27, 2018

at the risk of messing things up embarrassingly, I used the online editor to make a minor change to hopefully fix the travis failure

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ajkr is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@nvanbenschoten nvanbenschoten deleted the truncateEnv branch April 27, 2018 15:47
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this pull request Apr 27, 2018
facebook/rocksdb#3778 adds a DeleteRange method
to SstFileWriter and adds support for ingesting SSTs with range deletion
tombstones.

facebook/rocksdb#3779 adds a virtual Truncate method
to Env, which truncates the named file to the specified size.

Release note: None
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this pull request Apr 27, 2018
Fixes cockroachdb#16954.
Related to cockroachdb#25047.

This depends on the following two upstream changes to RockDB:
- facebook/rocksdb#3778
- facebook/rocksdb#3779

The change introduces a new snapshot strategy called "SST". This strategy
stream sst files consisting of all keys in a range from the sender to the
receiver. These sst files are then atomically ingested directly into RocksDB.
An important property of the strategy is that the amount of memory required
for a receiver using the strategy is constant with respect to the size of
a range, instead of linear as it is with the KV_BATCH strategy. This will
be critical for increasing the default range size and potentially for
increasing the number of concurrent snapshots allowed per node. The
strategy also seems to significantly speed up snapshots once ranges are
above a certain size (somewhere in the single digit MBs).

This is a WIP change. Before it can be merged it needs:
- to be cleaned up a bit
- more testing (unit test, testing knobs, maybe some chaos)
- proper version handling
- heuristic tuning
- decisions on questions like compactions after ingestion

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants