Commit Graph

151 Commits

Author SHA1 Message Date
krad
e01f32cc3c Parameterizing email id
Summary: Removing the leveldb@fb.com and making it a command line argument

Test Plan: Manual test

Reviewers:

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-24 15:18:36 -07:00
krad
628216fc1f Simplifying valgrind testing
Summary: Our valgrind testing is buggy and cumbersome in terms of locating the
error. It originates from the fact we accumulate all output for the tests. It is
extremely hard to locate the point of error.

The communication between valgrind and the script is not sturdy. We are
experiencing bugs.

Simplifying to stop on first error.

Test Plan: Run manually

Reviewers: sdong igor

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-23 15:08:58 -07:00
krad
03dd8f3ca7 Fixing punit job description
Summary: The job errors with log print step. Removing the step and re-enabling
the test case.

Test Plan: Manual test run

Reviewers:

CC: levledb@

Task ID: #6968635

Blame Rev:
2015-09-23 07:35:29 -07:00
krad
a6c22e3e41 Disabling parallel test CI job
Summary: Parallel test CI is failing. Reducing noise by temporarily disabling
the job

Test Plan: Manual

Reviewers:

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-22 10:37:56 -07:00
krad
dd2e1eeb35 Disabling log running jobs
Summary: Long running jobs are blocking the queue due to lack of adequate
machines in the CI

Test Plan: Manual testing

Reviewers:

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-21 12:58:33 -07:00
krad
4d6eb52d1a Fix to CI jobs.
Summary: The approach to serialize long scripts as string is flawed. Instead
adding args to invoke the long scripts.

Test Plan: Manual test

Reviewers:

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-19 17:33:06 -07:00
krad
3bcc072d24 Added more CI jobs
Summary: Migrating the long script jobs and the left overs from Jenkin.

Test Plan: Run command manually

Reviewers: sdon igor

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-18 13:24:09 -07:00
Andres Noetzli
1b598213aa Check formatting in Travis
Summary:
Check code formatting before running tests. Goal is to make it easier to deal
with external pull requests.

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

Reviewers: igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D47211
2015-09-17 22:02:05 -07:00
krad
2819a1db32 Minor fix to CI job definition
Summary: Timeout needs to be set for command and job for it to work.

Test Plan: Manual

Reviewers: sdong igor

CC: levledb@

Task ID: #6968635

Blame Rev:
2015-09-15 10:56:48 -07:00
Andres Noetzli
0bfe0573e8 Add gflags dependency to Travis script
Summary:
Travis is failing due to missing gflags
(https://travis-ci.org/facebook/rocksdb/jobs/80307921). Adding libgflags-dev
should help.

Test Plan: Run tests on Travis.

Reviewers: rven, anthony, yhchiang, aekmekji, igor, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D46905
2015-09-14 14:30:17 -07:00
krad
e2d6011f47 Minor fix to CI job definition
Summary: Command level timeouts are not working. Moving it to step level.

Test Plan: Manual testing sending commands

Reviewers: sdong igor

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-14 12:10:18 -07:00
krad
9f3a66a936 Improvements to CI jobs
Summary: Fixed
- Added timeouts for crash tests. They take around 9hrs.
- Added oncall so we can get notifications for timeout etc.
- Fixed a bug in the valgrind script
- Cosmetic fix

Test Plan: Manual run

Reviewers: sdong igor

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-11 16:09:01 -07:00
krad
7db1471cc1 Minor fix to CI job
Summary: Fix the issue where compilation error will not result in log file not
found error

Test Plan: None

Reviewers: sdong igor

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-11 13:00:55 -07:00
krad
41bce05869 CI job improvements
Summary: Added more jobs and refactored code express the jobs more cleanly

Test Plan: Manual test

Reviewers: igor sdong

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-10 14:29:41 -07:00
krad
52386a1e73 Minor fix to sandcastle jobs commands
Summary: Dropping parallel compilation since that seen to introduce compilation
errors spuriously

Test Plan: Run the command manually

Reviewers: sdon igor

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-09 10:54:50 -07:00
krad
778cf4449b Adding email notification.
Summary: The email notifications needs to be at command level and the job level.
Adding command level notification.

Test Plan: Run command manually

Reviewers: igor sdong

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-08 14:19:03 -07:00
krad
20ef64cae0 Moving jobs to use gcc-4.9
Summary: Fixed the glitch in Sandcastle and added gcc-4.9 support. Moving the
jobs to 4.9

Test Plan: Manually run the script

Reviewers: sdon igor

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-03 15:20:36 -07:00
krad
b8a962d4f5 Adding commands for few more CI jobs.
Summary: Added commands to spawn crash, stress, asan, asan_crash, clang CI jobs

Test Plan: Manual test

Reviewers: sdong

CC: leveldb@

Task ID: #6968635

Blame Rev:
2015-09-02 14:06:03 -07:00
krad
c6d870ffb0 Merge branch 'arcpatch-D45741' 2015-08-31 12:42:56 -07:00
krad
57b3a87731 Adding sandcastle determinator for RocksDB
Summary: This fuels commands to be executed for different sandcastle jobs. This is a nice way to separate RocksDB specific commands from Sandcastle specific job definition.

Test Plan: None. Will be tested when we add Sandcastle job.

Reviewers: igor, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D45741
2015-08-31 12:37:26 -07:00
sdong
7a0dbdf3ac Add ZSTD (not final format) compression type
Summary: Add ZSTD compression type. The same way as adding LZ4.

Test Plan: run all tests. Generate files in db_bench. Make sure reads succeed. But the SST files cannot be opened in older versions. Also some other adhoc tests.

Reviewers: rven, anthony, IslamAbdelRahman, kradhakrishnan, igor

Reviewed By: igor

Subscribers: MarkCallaghan, maykov, yoshinorim, leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D45747
2015-08-28 11:01:13 -07:00
Yoshinori Matsunobu
18ba58a943 Upgrading jemalloc from 3.6.0 to the latest for fbcode+gcc 4.8.1
Summary:
MyRocks is using jemalloc latest version, not 3.6.0.
Combining multiple versions (3.6.0 in RocksDB and latest in MyRocks)
broke some features -- for example, getting SIGSEGV when heap profiling
was enabled.
This diff switches to use jemalloc latest, if
env variable ROCKSDB_FBCODE_BUILD_WITH_481=1 was set.
My understanding is this env was used by MyRocks only so it would be
safe to change.

Test Plan: building MyRocks then verified jemalloc heap profiling worked

Reviewers: igor, rven, yhchiang, jtolmer, maykov, sdong

Reviewed By: sdong

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D43479
2015-08-04 16:35:26 -07:00
Igor Canadi
436ed904da Add rpath option to production builds for 4.8.1 toolchain
Summary: Copy change from D37533 to gcc 4.8.1 config

Test Plan: make db_bench, `ldd db_bench`, try running it

Reviewers: MarkCallaghan, anthony

Reviewed By: anthony

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D40845
2015-06-30 13:30:54 -07:00
Igor Canadi
09f5a4b486 set -e in fb_compile_mongo.sh
Summary: Based on @anthony's feedback, we want to fail early if our static linking fails.

Test Plan: none

Reviewers: anthony

Reviewed By: anthony

Subscribers: dhruba, anthony, leveldb

Differential Revision: https://reviews.facebook.net/D40839
2015-06-29 11:43:25 -07:00
Igor Canadi
0a019d74a0 Use malloc_usable_size() for accounting block cache size
Summary:
Currently, when we insert something into block cache, we say that the block cache capacity decreased by the size of the block. However, size of the block might be less than the actual memory used by this object. For example, 4.5KB block will actually use 8KB of memory. So even if we configure block cache to 10GB, our actually memory usage of block cache will be 20GB!

This problem showed up a lot in testing and just recently also showed up in MongoRocks production where we were using 30GB more memory than expected.

This diff will fix the problem. Instead of counting the block size, we will count memory used by the block. That way, a block cache configured to be 10GB will actually use only 10GB of memory.

I'm using non-portable function and I couldn't find info on portability on Google. However, it seems to work on Linux, which will cover majority of our use-cases.

Test Plan:
1. fill up mongo instance with 80GB of data
2. restart mongo with block cache size configured to 10GB
3. do a table scan in mongo
4. memory usage before the diff: 12GB. memory usage after the diff: 10.5GB

Reviewers: sdong, MarkCallaghan, rven, yhchiang

Reviewed By: yhchiang

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D40635
2015-06-26 11:48:09 -07:00
Igor Canadi
472e64d39e Improve fb_compile_mongo.sh
Summary: If we create a new temp directory for each build, scons will recompile everything because we have different parameters. Instead, let's set up a constant path to our static lib. That way we won't have to recompile.

Test Plan: Run fb_compile_mongo.sh twice -- second time it didn't recompile everything

Reviewers: MarkCallaghan, anthony

Reviewed By: anthony

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D40707
2015-06-26 10:24:08 -07:00
Igor Canadi
dadc429767 Reproducible MongoRocks compile with FB toolchain
Summary:
Added a script that will compile MongoRocks with the same flags as RocksDB binary. On FB infra, we can now do:

  cd ~/rocksdb; make static_lib
  cd ~/mongo; ~/rocksdb/build_tools/fb_compile_mongo.sh

No need to upgrade the g++ on the devbox (like Aaron and I did) or maintain a separate script to compile (like Mark did)

fb_compile_mongo.sh gets the settings from fbcode_config.sh, so it also makes it easier to upgrade the environment one day.

Test Plan: Compiled mongod with new script. Also, ldd output looks good: https://phabricator.fb.com/P19891602

Reviewers: AaronFeldman, MarkCallaghan, anthony

Reviewed By: anthony

Subscribers: anthony, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D40659
2015-06-24 15:09:55 -07:00
Yueh-Hsuan Chiang
0b1ffe2e1d Remove -Wl,--no-as-needed flag when making shared_lib in OSX and IOS
Summary:
Remove -Wl,--no-as-needed flag when making shared_lib in OSX and IOS as
those environment doe not have compile option --no-as-needed

  ld: unknown option: --no-as-needed
  clang: error: linker command failed with exit code 1 (use -v to see invocation)

Test Plan: make shared_lib

Reviewers: meyering, igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D40353
2015-06-23 16:32:59 -07:00
Igor Canadi
c89369f57c Move dockerbuild.sh to build_tools/
Summary: That's where we keep build tools :)

Test Plan: none

Reviewers: sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D39741
2015-06-17 14:09:12 -07:00
sdong
46296cc869 Cygwin build not to use -fPIC
Summary:
Cygwin doesn't support -fPIC. Remove it.
Not sure whether we can build shared library in Cygwin but at least it can build without warning.

Test Plan: Build under Cygwin

Reviewers: yhchiang, rven, kradhakrishnan, igor

Reviewed By: igor

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D40077
2015-06-12 13:59:59 -07:00
sdong
98a44559d5 Build for CYGWIN
Summary:
Make it build for CYGWIN.
Need to define "-std=gnu++11" instead of "-std=c++11" and use some replacement functions.

Test Plan: Build it and run some unit tests in CYGWIN

Reviewers: yhchiang, rven, anthony, kradhakrishnan, igor

Reviewed By: igor

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D37605
2015-04-23 21:33:44 -07:00
Igor Canadi
fd7a357318 Enable open source users to use jemalloc (github issue #438)
Summary: Currently open source rocksdb only builds with tcmalloc. This diff first checks if jemalloc is available. If it is, it compiles with jemalloc. If it isn't, it checks for tcmalloc.

Test Plan: Tried this out on my Ubuntu virtual machine and confirms that jemalloc is correctly detected and compiled.

Reviewers: MarkCallaghan, yhchiang, rven, sdong

Reviewed By: sdong

Subscribers: adamretter, meyering, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36789
2015-04-23 17:48:18 -07:00
Mark Callaghan
6e359419fe Add rpath for production builds
Summary:
This lets the production toolchain libraries get used on devservers and
in production.

Task ID: #6849362

Blame Rev:

Test Plan:
Revert Plan:

Database Impact:

Memcache Impact:

Other Notes:

EImportant:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Reviewers: igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37533
2015-04-22 17:17:50 -07:00
Igor Canadi
6a5ffee0cc Fix gflags Makefile
Summary: `echo` correctly interpretes \n on mac, but not on linux. On linux you have to give it `-e` to interpret \n. Unfortunately, `-e` options is not available on Mac. Go back to old way of checking gflags

Test Plan: build_tools/build_detect_platform on mac and linux

Reviewers: sdong

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37515
2015-04-22 12:50:28 -07:00
Igor Canadi
7d136994c9 Get rid of error output
Summary: We should send error output to /dev/null

Test Plan: none

Reviewers: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37449
2015-04-20 19:44:32 -07:00
Igor Canadi
742fa9e316 Fix compile with two gflags
Summary:
If the system has gflags with both `google` and `gflags` namespaces installed, we try to define GFLAGS as two things. This breaks the compile.

Fix: Use `else if` -- try compiling with `google` namespace only if compile with `gflags` failed

Test Plan: build_tools/build_detect_platform correctly identifies gflags

Reviewers: lgalanis

Reviewed By: lgalanis

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37389
2015-04-20 10:55:17 -07:00
Igor Canadi
98ef21d2ff Merge pull request #584 from pshareghi/rocksdb-3.10-falloch
Added falloc.h in build_detect_platform
2015-04-15 14:56:36 -07:00
Pooya Shareghi
e8808b9128 Added falloc.h in build_detect_platform
On Centos 6, you need to explicitely include linux/falloc.h which is
whele the  FALLOC_FL_* flags are defined. Otherwise, the fallocate()
support test defined in build_detect_platform will fail.

Signed-off-by: Pooya Shareghi <shareghi@gmail.com>
2015-04-13 17:56:12 -07:00
Igor Canadi
91df4e969d Remove use of whole-archive to include jemalloc
Summary: I don't think we need to use whole-archive to include jemalloc. This change only affects our development builds -- it does not affect our open source builds (which don't support jemalloc) or our fbcode third-party2 builds (which use open-source build codepaths).

Test Plan:
make
verify that jemalloc is running by running `MALLOC_CONF="prof:true" ./cache_test` and observing that file was created

Reviewers: MarkCallaghan

Reviewed By: MarkCallaghan

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36783
2015-04-09 15:10:53 -07:00
Igor Canadi
c66483c132 Fix github issue #563
Summary:
As described in https://github.com/facebook/rocksdb/issues/563, we should add minor version to SONAME, since we break ABI with minor releases.

I also turned PLATFORM_SHARED_VERSIONED to true by default. This is true in LevelDB and it was switched to false by D15117 for no apparent reason. It should only be false for iOS.

Test Plan: `make shared_lib` produced librocksdb.dylib.3.10.0

Reviewers: sdong, yhchiang, meyering

Reviewed By: meyering

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36573
2015-04-07 13:22:22 -07:00
Igor Canadi
de22c7bd1f Integrate Jenkins with Phabricator
Summary:
After this diff, when a user submits a diff from Facebook's VPN
network, we'll automatically trigger a jenkins test. Once jenkins test
is done, we'll update the diff with test results.

Test Plan:
Made sure that jenkins build is triggered on `arc diff` and
that result is reflected back on the diff

Reviewers: sdong, rven, kradhakrishnan, anthony, yhchiang

Reviewed By: anthony

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D36555
2015-04-07 11:56:29 -07:00
Igor Canadi
df71c6b9ed Script to trigger jenkins test
Summary: After you run `arc diff`, just run `build_tools/trigger_jenkins_test.sh` and Jenkins will test your diff!

Test Plan: Triggered a build to jenkins

Reviewers: sdong, rven, IslamAbdelRahman, anthony, yhchiang, meyering

Reviewed By: meyering

Subscribers: meyering, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36021
2015-03-31 08:56:40 -07:00
Igor Sugak
67d995808a rocksdb: fix make unity
Summary:
changed make unity target to use $LIB_SOURCES as a source of library source code. In the old way (using find) table/mock_table.h was added to a list of library objects and this was a course of `make unity` break.

`build_tools/unity` contains some redundant code, I deleted it and moved the functionality in the Makefile.

Test Plan:
Make sure unity completes with no errors.
```lang=bash
% make unity
```

Reviewers: sdong, rven, igor, meyering

Reviewed By: igor, meyering

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D35385
2015-03-18 19:12:23 -07:00
Igor Sugak
bd4963e64d rocksdb: update reference to the latest version of clang dev in fbcode
Summary: Updated reference to the latest clang dev in fbcode. Since RocksDB already uses recent version of clang dev no code changes need.

Test Plan:
Make sure can be build with clang.
```lang=bash
% USE_CLANG=1 make all
```

Reviewers: meyering, sdong, rven, igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D35121
2015-03-16 17:08:05 -07:00
Venkatesh Radhakrishnan
480b284766 Fix make_new_version.sh
Summary:
One of the commands in the script make_new_version.sh was
incorrect. Fixed it.

Test Plan: Try the script out to verify that it works.

Reviewers: igor, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D34791
2015-03-10 17:24:33 -07:00
Jim Meyering
ebc647de87 build: fix missing dependency problems
Summary:
Any time one would modify a dependent of any *test*.cc file,
"make" would fail to rebuild the affected test binaries,
e.g., db_test.  That was due to the fact that we deliberately
excluded those test-related files from the definition of SOURCES
and only $(SOURCES) was used to create the automatically-generated
.d dependency files.  The fix is to generate a .d file for every
source file.
* src.mk: New file.  Defines LIB_SOURCES, MOCK_SOURCES
and TEST_BENCH_SOURCES.
* Makefile: Include src.mk.
Reflect s/SOURCES/LIB_SOURCES/ renaming.
* build_tools/build_detect_platform: Remove the code
that was used to generate SOURCES= and MOCK_SOURCES=
definitions in make_config.mk. Those lists of files
are now hard-coded in src.mk. Hard-coding this list of
sources is desirable, because without that, one risks
including stray .cc files in a build.  Not reproducible.

Test Plan:
Touch a file used by db_test's dependent .o files and ensure that
they are all recompiled.  Before, none would be:

  $ touch db/db_impl.h && make db_test
    CC       db/db_test.o
    CC       db/column_family.o
    CC       db/db_filesnapshot.o
    CC       db/db_impl.o
    CC       db/db_impl_debug.o
    CC       db/db_impl_readonly.o
    CC       db/forward_iterator.o
    CC       db/internal_stats.o
    CC       db/managed_iterator.o
    CC       db/repair.o
    CC       db/write_batch.o
    CC       utilities/compacted_db/compacted_db_impl.o
    CC       utilities/ttl/db_ttl_impl.o
    CC       util/ldb_cmd.o
    CC       util/ldb_tool.o
    CC       util/sst_dump_tool.o
    CC       util/xfunc.o
    CCLD     db_test

Reviewers: ljin, igor.sugak, igor, rven, sdong

Reviewed By: sdong

Subscribers: yhchiang, adamretter, fyrz, dhruba

Differential Revision: https://reviews.facebook.net/D33849
2015-03-06 10:55:11 -08:00
Igor Canadi
f9c14a42e8 Fix compile on Mac 2015-02-27 09:31:29 -08:00
Islam AbdelRahman
ba9d1737a8 RocksDB on FreeBSD support
Summary:
This patch will update the Makefile and source code so that we can build RocksDB successfully on FreeBSD 10 and 11 (64-bit and 32-bit)
I have also encountered some problems when running tests on FreeBSD, I will try to fix them individually in different diffs

Notes:

  - FreeBSD uses clang as it's default compiler (http://lists.freebsd.org/pipermail/freebsd-current/2012-September/036480.html)
  - GNU C++ compiler have C++ 11 problems on FreeBSD (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=193528)
  - make is not gmake on FreeBSD (http://www.khmere.com/freebsd_book/html/ch01.html)

Test Plan:
Using VMWare Fusion Create 4 VM machines (FreeBSD 11 64-bit, FreeBSD 11 32-bit, FreeBSD 10 64-bit, FreeBSD 10 32-bit)

  - pkg install git gmake gflags archivers/snappy
  - git clone https://github.com/facebook/rocksdb.git
  - apply this patch
  - setenv CXX c++
  - setenv CPATH /usr/local/include/
  - setenv LIBRARY_PATH  /usr/local/lib/
  - gmake db_bench
  - make sure compilation is successful and db_bench is running
  - gmake all
  - make sure compilation is successful

Reviewers: sdong, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D33891
2015-02-26 15:19:17 -08:00
Igor Sugak
4ba119df5c rocksdb: Update path to dev clang in fbcode
Summary: Current clang path in fbcode points to clang 3.5. This diff updates clang path to clang 3.7.

Test Plan:
Make sure clang 3.7 is in use and all tests are passing.
```lang=bash
% USE_CLANG make check
```

Reviewers: meyering, sdong, rven, igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D34119
2015-02-26 15:12:08 -08:00
Mark Callaghan
1e06a40685 Support builds for MongoDB+RocksDB
Summary:
This changes the RocksDB build to share headers provided by MongoDB. It is invoked as:
  ROCKSDB_FOR_MONGO=/path/to/mongodb/root make static_lib

Task ID: #

Blame Rev:

Test Plan:
Revert Plan:

Database Impact:

Memcache Impact:

Other Notes:

EImportant:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Reviewers: igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D34071
2015-02-26 10:50:03 -08:00