Commit Graph

5695 Commits

Author SHA1 Message Date
Joel Marcey
2adab1dde2 Add API links to the header bar
Summary:
Until we get new API infra for gh-pages, this will point to the C++ headers and Java
files, respectively for API information.

Test Plan:
Visual

https://www.facebook.com/pxlcld/pwzQ

Reviewers: lgalanis, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63771
2016-09-13 09:13:59 -07:00
Joel Marcey
a182b2981d Preserve blog comments in markdown
Summary:
While we won't have new comments when we move to gh-pages (for now), this preserves
the current comments in markdown format.

Test Plan:
Visual

https://www.facebook.com/pxlcld/pwCR

Reviewers: lgalanis, sdong

Reviewed By: sdong

Subscribers: jamesgpearce, andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63783
2016-09-13 09:13:29 -07:00
Pawan Rawal
f54de92307 Adding Dgraph to list of Users (#1291)
* Adding Dgraph

* open-source
2016-09-12 17:33:44 -07:00
somnathr
9e4aa798c3 Summary: (#1313)
If log recycling is enabled with the rocksdb (recycle_log_file_num=16)
 db->Writebatch is erroring out with keynotfound after ~5-6 hours of run
 (1M seq but can happen to any workload I guess).See my detailed bug
 report here (https://github.com/facebook/rocksdb/issues/1303).
 This commit is the fix for this, a check is been added not to delete
 the log file if it is already there in the recycle list.

Test Plan:
 Unit tested it and ran the similar profile. Not reproducing anymore.
2016-09-12 16:53:42 -07:00
Adam Faulkner
a10e8a056d Fix C api memtable rep bugs. (#1328) 2016-09-12 15:31:42 -07:00
Islam AbdelRahman
eb1d4d53c8 Release RocksDB 4.12
Summary: Release 4.12

Test Plan: none

Reviewers: andrewkr, yiwu, lightmark, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63885
2016-09-12 12:34:08 -07:00
Adam Retter
22d88e24db Allow an offset as well as a length to be specified for byte[] operations in RocksJava JNI (#1264)
Test Plan: Execute the Java test suite

Reviewers: yhchiang

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D61971
2016-09-12 11:51:08 -07:00
zhangjinpeng1987
b06b191362 add C api for set wal_recovery_mode (#1327)
* add C api for set wal recovery mode

* add test
2016-09-09 10:11:30 -07:00
Islam AbdelRahman
1cca091298 Temporarily revert Prev() prefix support
Summary:
Temporarily revert commits for supporting prefix Prev() to unblock MyRocks and RocksDB release

These are the commits reverted

  - 6a14d55bd9
  - b18f9c9eac
  - db74b1a219
  - 2482d5fb45

Test Plan: make check -j64

Reviewers: sdong, lightmark

Reviewed By: lightmark

Subscribers: andrewkr, dhruba, yoshinorim

Differential Revision: https://reviews.facebook.net/D63789
2016-09-08 14:45:32 -07:00
Andrew Kryczka
de28a25533 Update HISTORY.md for thread-local stats
Summary: as titled

Test Plan: doitlive

Reviewers: sdong, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63777
2016-09-08 14:02:19 -07:00
ammongit
0fcb6dbed7 Remove extraneous function prototypes from c.h (#1326)
* Fix function prototypes from upstream commit 32149059.

* Fix removed function.

* Readd removed function.
2016-09-08 11:31:06 -07:00
Islam AbdelRahman
52ee07b021 Move AddFile() tests to external_sst_file_test.cc
Summary: Simply move the tests

Test Plan: make check -j64

Reviewers: andrewkr, lightmark, yiwu, yhchiang, kradhakrishnan, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D62529
2016-09-07 15:41:54 -07:00
Edouard A
66a91e2607 Add NoSpace subcode to IOError (#1320)
Add a sub code to distinguish "out of space" errors from regular I/O errors
2016-09-07 12:37:45 -07:00
sdong
67036c0406 Fix Flaky ColumnFamilyTest.FlushCloseWALFiles
Summary: In ColumnFamilyTest.FlushCloseWALFiles, there is a small window in which the flush has finished but the log writer is not yet closed, causing the assert failure. Fix it by explicitly waiting the flush job to finish.

Test Plan: Run the test many times in high parallelism.

Subscribers: leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63423
2016-09-07 11:19:15 -07:00
wenduo
0e2da497c6 fix typo in option.h's comment (#1321) 2016-09-07 01:36:16 -07:00
Joel Marcey
6d61358a09 Add real Google Analytics ID
Summary:
This adds the actual RocksDB Google Analytics ID that will be used when
we switch over from WP to GH-pages

Test Plan: visual

Reviewers: IslamAbdelRahman, lgalanis, sdong

Reviewed By: sdong

Subscribers: sdong, andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63429
2016-09-06 21:10:14 -07:00
Joel Marcey
2d9d36ea46 Have "Edit on GitHub" point to master instead of gh-pages
Summary:
If someone clicks on `Edit on GitHub` to edit a doc for a pull request, for example,
then we should point to `master` instead of `gh-pages`.

> Also fixed some Windows-based line endings. From CRLF to LF.

Test Plan: https://www.facebook.com/pxlcld/pvV2

Reviewers: IslamAbdelRahman, lgalanis, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63435
2016-09-06 21:08:53 -07:00
Joel Marcey
9377518981 Update landing page content
Summary: Changes based on comments in D62985

Test Plan:
Visual

https://www.facebook.com/pxlcld/pvVs

Reviewers: lgalanis, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63477
2016-09-06 21:07:47 -07:00
Joel Marcey
1ec75ee76b Add redirects from old blog posts link to new format
Summary:
The new blog post links will be formatted differently coming over to gh-pages. But
we can redirect from the old style over to the new style for existing blog posts.

Test Plan:
Visual

https://www.facebook.com/pxlcld/pvWQ

Reviewers: lgalanis, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63513
2016-09-06 21:07:13 -07:00
sdong
607628d349 Support ZSTD with finalized format
Summary:
ZSTD 1.0.0 is coming. We can finally add a support of ZSTD without worrying about compatibility.
Still keep ZSTDNotFinal for compatibility reason.

Test Plan: Run all tests. Run db_bench with ZSTD version with RocksDB built with ZSTD 1.0 and older.

Reviewers: andrewkr, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: cyan, igor, IslamAbdelRahman, leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63141
2016-09-06 12:22:16 -07:00
Injun Song
ce1be2ce37 Fix build error on Windows (AppVeyor) (#1315)
Add 'cf_options' to source list and db_imple.cc

fix casting
2016-09-06 08:41:43 -07:00
sdong
f7669b40ba Fix Windows Build
Summary: Fix two Windows build problems.

Test Plan: Build on Windows and run all Linux tests.

Reviewers: IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63189
2016-09-02 17:10:28 -07:00
sdong
22696b0881 Fix uninitlized CompactionJob::SubcompactionState::current_output_file_size
Summary: The new variable introduced in 2149059f910149197d1a0f79ac08cf19465ea2d may be unitialized. Valgrind is failing because of it.

Test Plan: Run valgrind tests

Reviewers: yiwu, andrewkr, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63201
2016-09-02 17:06:20 -07:00
Islam AbdelRahman
c1865e0f7f Trigger more tests per diff
Summary: We trigger tests for diffs we land, we should trigger as much as possible to make our dashboard as recent as possible

Test Plan: none

Reviewers: gunnarku, kradhakrishnan, andrewkr, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63159
2016-09-02 16:30:47 -07:00
Yi Wu
a88677d2cf Remove ImmutableCFOptions from public API
Summary: There's no reference to ImmutableCFOptions elsewhere in /include/rocksdb. ImmutableCFOptions was introduced in this commit (5665e5e285) but later its reference in /include/rocksdb/table.h is removed.

Test Plan:
  make all check

Reviewers: IslamAbdelRahman, sdong, yhchiang

Reviewed By: yhchiang

Subscribers: yhchiang, andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63177
2016-09-02 14:16:31 -07:00
Islam AbdelRahman
80c75593ed Fix data race in AddFile() with multiple files + custom comparator bug
Summary:
When ingesting multiple files
- We should use user comparator
- Should not call `cfd->current()` outside of mutex

Test Plan: unit tests

Reviewers: sdong, lightmark

Reviewed By: lightmark

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63075
2016-09-02 11:17:58 -07:00
Islam AbdelRahman
5051755e35 Fix db_bench memory use after free (detected by clang_analyze)
Summary: Fix using `arg[i].thread` after deleting it

Test Plan: run clang_analyze

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63171
2016-09-02 10:58:08 -07:00
John Alexander
4fd08f4b8b Ensure Correct Behavior of StatsLevel kExceptDetailedTimers and kExceptTimeForMutex (#1308)
* Fix StatsLevel so that kExceptTimeForMutex leaves compression stats enabled and kExceptDetailedTimers disables mutex lock stats. Also change default stats level to kExceptDetailedTimers (disabling both compression and mutex timing).

* Changed order of StatsLevel enum to simplify logic for determining what stats to record.
2016-09-01 19:57:55 -07:00
Joel Marcey
e14fbaae26 Add FAQ based on the front page of the current rocksdb.org
Summary:
This takes the questions currently on the front of the Wordpress-based rocksdb.org and starts an official FAQ with them.

Depends on: https://reviews.facebook.net/D63105

Test Plan: Visual

Reviewers: IslamAbdelRahman, lgalanis

Reviewed By: lgalanis

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63123
2016-09-01 17:34:39 -07:00
Joel Marcey
3c2262400f Migrate the RocksDB Worpdress blog over to Jekyll
Summary:
Tried to:

- preserve existing links
- move existing images over (there were 2)
- preserve codeblocks (modified where apprporiate)
- etc.

Also as agreed upon:

- All blog posts are preserved.
- Comments are not preserved.
- Not turning on comments for future blog posts (use the FB developer group instead).
- Like button at the end of the blog post.

Depends on https://reviews.facebook.net/D63051

Test Plan: Visual

Reviewers: IslamAbdelRahman, lgalanis

Reviewed By: lgalanis

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63105
2016-09-01 17:28:49 -07:00
Joel Marcey
ee0e2201e0 Transfer the current Getting Started contents to GitHub Pages
Summary:
This moves the current Getting Started that we have hosted over to our new GitHub pages infra.

Next will be to move the blog over, along with some updates and maintenance of links, etc. in the new infra.

Depends on: https://reviews.facebook.net/D62985

Test Plan: Visual

Reviewers: IslamAbdelRahman, lgalanis

Reviewed By: lgalanis

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63051
2016-09-01 17:26:38 -07:00
Joel Marcey
5a0e9a4cfa Initial Landing Page
Summary:
This is the proof of concept for the initial landing page for RocksDB when we move to GitHub pages.

The images for each feature can change, but using these as placeholders for now.

The styling may change a bit as well.

Test Plan: Visual

Reviewers: IslamAbdelRahman, lgalanis, sdong

Reviewed By: lgalanis

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62985
2016-09-01 17:25:08 -07:00
Joel Marcey
9447a85404 Remove the doc directory
Summary: The content in this directory is old, has not been updated in some cases years, and superceded by the current Wordpress-based docs (soon to be changed to Jekyll)

Test Plan: visual

Reviewers: IslamAbdelRahman, lgalanis

Reviewed By: lgalanis

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62877
2016-09-01 15:24:19 -07:00
sdong
32149059f9 Merge options source_compaction_factor, max_grandparent_overlap_bytes and expanded_compaction_factor into max_compaction_bytes
Summary: To reduce number of options, merge source_compaction_factor, max_grandparent_overlap_bytes and expanded_compaction_factor into max_compaction_bytes.

Test Plan: Add two new unit tests. Run all existing tests, including jtest.

Reviewers: yhchiang, igor, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D59829
2016-09-01 14:33:24 -07:00
Aaron Gao
4590b53a4b add stats to Cache::LookUp()
Summary: basically for SimCache stats. I find most times it is hard to pass Statistics* to SimCache constructor.

Test Plan: make all check

Reviewers: andrewkr, sdong, yiwu

Reviewed By: yiwu

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62193
2016-09-01 13:50:39 -07:00
Sherlock
85bb308258 Expose Utility function StringToMap() (#1306) 2016-09-01 13:47:21 -07:00
Islam AbdelRahman
8ce1b8440a Fix Travis on Mac
Summary: not sure why travis complain about this line, works fine on my mac

Test Plan: run on my mac

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D63045
2016-08-31 15:10:12 -07:00
Kien-hung Li
380e651afc Fix Mac build failure (#1309) 2016-08-31 15:03:10 -07:00
Andrew Kryczka
1613fa9490 Thread-specific histogram statistics
Summary:
To reduce contention for atomics when HistogramStats are shared across
threads, this diff makes them thread-specific so updates are faster. This comes
at the expense of slower reads (much less frequent), which now require merging
all histograms. In this diff,

- Thread-specific HistogramImpl is created upon the thread's first measureTime()
- Thread-specific HistogramImpl are merged and deleted upon thread termination or ThreadLocalPtr destruction, whichever comes first
- getHistogramString() and histogramData() merge all histograms, both thread-specific and previously merged ones

Test Plan:
unit tests, ran db_bench and verified histograms look similar

before:

  $ TEST_TMPDIR=/dev/shm/ perf record -g ./db_bench --benchmarks=readwhilewriting --statistics --num=1000000 --use_existing_db --threads=64 --cache_size=250000000 --compression_type=lz4
  ...
  +    7.63%  db_bench     db_bench             [.] rocksdb::HistogramStat::Add

after:

  $ TEST_TMPDIR=/dev/shm/ perf record -g ./db_bench --benchmarks=readwhilewriting --statistics --num=1000000 --use_existing_db --threads=64 --cache_size=250000000 --compression_type=lz4
  ...
  +    0.98%  db_bench     db_bench             [.] rocksdb::HistogramStat::Add

Reviewers: sdong, MarkCallaghan, kradhakrishnan, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62649
2016-08-31 14:02:09 -07:00
Aaron Gao
6a14d55bd9 add prefix_seek_mode to db_iter_test
Summary: add prefix_seek_mode to db_iter_test to enable data race test for iterator when prefix_extractor != nullptr

Test Plan: make all check -j64

Reviewers: andrewkr, IslamAbdelRahman, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D63027
2016-08-31 12:07:09 -07:00
Yi Wu
de47e2bd4d Fix ClockCache memory leak
Summary:
Fix ClockCache memory leak found by valgrind:
# Add destructor to cleanup cached values.
# Delete key with cache handle immediately after handle is recycled, and erase table entry immediately if duplicated cache entry is inserted.

Test Plan:
    make DISABLE_JEMALLOC=1 valgrind_check

Reviewers: IslamAbdelRahman, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62973
2016-08-31 08:56:34 -07:00
Islam AbdelRahman
f099af4c76 Fix travis
Summary: Fix travis for mac by using gflags package instead of doing `brew install`

Test Plan: https://travis-ci.org/facebook/rocksdb/builds/156358515

Reviewers: sdong, andrewkr, lightmark, yiwu

Reviewed By: yiwu

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D62997
2016-08-31 00:10:49 -07:00
Aaron Gao
db74b1a219 fix bug in merge_iterator when data race happens
Summary:
core dump when run
`./db_stress --max_background_compactions=1 --max_write_buffer_number=3 --sync=0 --reopen=20 --write_buffer_size=33554432 --delpercent=5 --log2_keys_per_lock=10 --block_size=16384 --allow_concurrent_memtable_write=1 --test_batches_snapshots=0 --max_bytes_for_level_base=67108864 --progress_reports=0 --mmap_read=1 --kill_prefix_blacklist=WritableFileWriter::Append,WritableFileWriter::WriteBuffered --writepercent=35 --disable_data_sync=0 --readpercent=50 --subcompactions=3 --ops_per_thread=20000000 --memtablerep=skip_list --prefix_size=0 --target_file_size_multiplier=1 --column_families=1 --db=/dev/shm/rocksdb/rocksdb_crashtest_whitebox --threads=32 --disable_wal=0 --open_files=500000 --destroy_db_initially=0 --target_file_size_base=16777216 --nooverwritepercent=1 --iterpercent=10 --max_key=100000000 --prefixpercent=0 --use_clock_cache=false --kill_random_test=189 --cache_size=1048576 --verify_checksum=1`
Actually the relevant flag is `--threads`, data race when --thread > 1 cause problem.
It is possible that multiple
threads read/write memtable simultaneously. After one thread
calls Prev(), another thread may insert a new key just between
the current key and the key next, which may cause the
assert(current_ == CurrentForward()) failure when the first
thread calls Next() again if in prefix seek mode

Test Plan: rerun db_stress with >1 thread / make all check -j64

Reviewers: sdong, andrewkr, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62979
2016-08-30 22:19:42 -07:00
Aaron Gao
b18f9c9eac add nullptr check to internal_prefix_transform
Summary: patch for D62361

Test Plan: make all check

Reviewers: sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62883
2016-08-30 13:48:31 -07:00
Joel Marcey
4e395e875e Update docs README.md
Summary: Update the docs `README.md` to be more specific to RocksDB as opposed to the more generic information that is there now.

Test Plan: visual

Reviewers: lgalanis, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62733
2016-08-30 08:05:43 -07:00
Aaron Gao
2482d5fb45 support Prev() in prefix seek mode
Summary: As title, make sure Prev() works as expected with Next() when the current iter->key() in the range of the same prefix in prefix seek mode

Test Plan: make all check -j64 (add prefix_test with PrefixSeekModePrev test case)

Reviewers: andrewkr, sdong, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: yoshinorim, andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D61419
2016-08-29 20:55:39 -07:00
Yi Wu
7541c7a79e Fix cache_test valgrind_check failure
Summary: Refactor cache_test to get around gtest valgrind failure.

Test Plan:
    make valgrind_check

Reviewers: sdong, kradhakrishnan, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D62817
2016-08-29 10:40:00 -07:00
Adam Retter
c8513cde0c Update the download location of Snappy (#1304)
* Fix the download location of Snappy, no longer available from Google Code

* Ensure that curl follows any redirect headers when downloading binaries
2016-08-28 20:24:08 -07:00
Islam AbdelRahman
b49b92cf28 Introduce Read amplification bitmap (read amp statistics)
Summary:
Add ReadOptions::read_amp_bytes_per_bit option which allow us to create a bitmap for every data block we read
the bitmap will contain (block_size / read_amp_bytes_per_bit) bits.

We will use this bitmap to mark which bytes have been used of the block so we can calculate the read amplification

Test Plan: added new tests

Reviewers: andrewkr, yhchiang, sdong

Reviewed By: sdong

Subscribers: yiwu, leveldb, march, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D58707
2016-08-26 18:55:58 -07:00
Aaron Gao
c7004840d2 store prefix_extractor_name in table
Summary:
Make sure prefix extractor name is stored in SST files and if DB is opened with a prefix extractor of a different name, prefix bloom is skipped when read the file.
Also add unit tests for that.

Test Plan:
before change:
```
Note: Google Test filter = BlockBasedTableTest.SkipPrefixBloomFilter
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from BlockBasedTableTest
[ RUN      ] BlockBasedTableTest.SkipPrefixBloomFilter
table/table_test.cc:1421: Failure
Value of: db_iter->Valid()
Actual: false
Expected: true
[  FAILED  ] BlockBasedTableTest.SkipPrefixBloomFilter (1 ms)
[----------] 1 test from BlockBasedTableTest (1 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (1 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] BlockBasedTableTest.SkipPrefixBloomFilter

1 FAILED TEST
```
after:
```
Note: Google Test filter = BlockBasedTableTest.SkipPrefixBloomFilter
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from BlockBasedTableTest
[ RUN      ] BlockBasedTableTest.SkipPrefixBloomFilter
[       OK ] BlockBasedTableTest.SkipPrefixBloomFilter (0 ms)
[----------] 1 test from BlockBasedTableTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (0 ms total)
[  PASSED  ] 1 test.
```

Reviewers: sdong, andrewkr, yiwu, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D61215
2016-08-26 11:46:32 -07:00