Summary:
Various renaming and fixes to get rid of remaining uses of
"backupable" which is terminology leftover from the original, flawed
design of BackupableDB. Now any DB can be backed up, using BackupEngine.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9792
Test Plan: CI
Reviewed By: ajkr
Differential Revision: D35334386
Pulled By: pdillinger
fbshipit-source-id: 2108a42b4575c8cccdfd791c549aae93ec2f3329
Summary:
Make `commit_prereq` work and a few other improvements:
* Remove gcc 481 and gcc5xx which are no longer supported
* Remove platform007 which is gone
* `make clean` work for both mac and linux
* `precommit_checker.py` to python3
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9797
Test Plan: `make commit_prereq`
Reviewed By: ajkr
Differential Revision: D35338536
Pulled By: jay-zhuang
fbshipit-source-id: 1e159962ab9d31c43c4b85de7d0f582d3e881ffe
Summary:
Right now, parallelism information passed to "build_tools/rocksdb-lego-determinator no_compression" isn't effective when the test actually runs, as the information is dropped in the middle. Fix it.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9796
Test Plan: Run "build_tools/rocksdb-lego-determinator no_compression" and execute the command line generated and observe the parallelism.
Reviewed By: jay-zhuang
Differential Revision: D35330085
fbshipit-source-id: e9b32d0520d61fbc2697ebd841099485f64482e3
Summary:
build_tools/rocksdb-lego-determinator is to generate commands for continuous tests. Recently it changed to by default run tests in parallel with parallelism to be number of CPU processors. This sometimes causes out of space when running so many tests in parallel. Reduce the parallelism by half to temporarily work it around.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9788
Test Plan: Run build_tools/rocksdb-lego-determinator and watch generated commands.
Reviewed By: pdillinger
Differential Revision: D35327704
fbshipit-source-id: 95a8c51a111bb6ab62c456c74ab9c905b457ea8f
Summary:
So the build on dev server will work.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9787
Test Plan: `$ make db_basic_bench` on dev server.
Reviewed By: ajkr
Differential Revision: D35295466
Pulled By: jay-zhuang
fbshipit-source-id: 58dccc65bc29e1185b97cbeb7630ed66deb604aa
Summary:
Fix g++ -march=native detection and reenable s390x in travis
This PR fixes s390x assembler messages:
```
Error: invalid switch -march=z14
Error: unrecognized option -march=z14
```
The s390x travis build was failing with gcc-7 because the assembler on
ubuntu 16.04 is too old to recognize the z14 model so it doesn't work
with -march=native on a z14 machine. It fixes the check for the
-march=native flag so that the assembler will get called and correctly
fail on ubuntu 16.04 which will cause the build to fall back to
-march=z196 which works.
The other changes are needed so builds work more consistently on
s390x:
1. Set make parallelism to 1 for s390x: The default was 4 previously
but I saw frequent internal compiler errors on travis probably due to
low resources. The `platform_dependent` job works more consistently
but is roughly 10 minutes slower although it varies.
2. Remove status_checked jobs, as we are relying on CircleCI for
these now and do not really need platform coverage on them.
Fixes https://github.com/facebook/rocksdb/issues/9524
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9631
Test Plan: CI
Reviewed By: ajkr
Differential Revision: D34553989
Pulled By: pdillinger
fbshipit-source-id: a6e3a7276446721c4c0bebc4ed217c2ca2b53f11
Summary:
Related to: https://github.com/facebook/rocksdb/pull/9215
* Adds build_detect_platform support for RISCV on Linux (at least on SiFive Unmatched platforms)
This still leaves some linking issues on RISCV remaining (e.g. when building `db_test`):
```
/usr/bin/ld: ./librocksdb_debug.a(memtable.o): in function `__gnu_cxx::new_allocator<char>::deallocate(char*, unsigned long)':
/usr/include/c++/10/ext/new_allocator.h:133: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: ./librocksdb_debug.a(memtable.o): in function `std::__atomic_base<bool>::compare_exchange_weak(bool&, bool, std::memory_order, std::memory_order)':
/usr/include/c++/10/bits/atomic_base.h:464: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: /usr/include/c++/10/bits/atomic_base.h:464: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: /usr/include/c++/10/bits/atomic_base.h:464: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: /usr/include/c++/10/bits/atomic_base.h:464: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: ./librocksdb_debug.a(memtable.o):/usr/include/c++/10/bits/atomic_base.h:464: more undefined references to `__atomic_compare_exchange_1' follow
/usr/bin/ld: ./librocksdb_debug.a(db_impl.o): in function `rocksdb::DBImpl::NewIteratorImpl(rocksdb::ReadOptions const&, rocksdb::ColumnFamilyData*, unsigned long, rocksdb::ReadCallback*, bool, bool)':
/home/adamretter/rocksdb/db/db_impl/db_impl.cc:3019: undefined reference to `__atomic_exchange_1'
/usr/bin/ld: ./librocksdb_debug.a(write_thread.o): in function `rocksdb::WriteThread::Writer::CreateMutex()':
/home/adamretter/rocksdb/./db/write_thread.h:205: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: ./librocksdb_debug.a(write_thread.o): in function `rocksdb::WriteThread::SetState(rocksdb::WriteThread::Writer*, unsigned char)':
/home/adamretter/rocksdb/db/write_thread.cc:222: undefined reference to `__atomic_compare_exchange_1'
collect2: error: ld returned 1 exit status
make: *** [Makefile:1449: db_test] Error 1
```
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9366
Reviewed By: jay-zhuang
Differential Revision: D34377664
Pulled By: mrambacher
fbshipit-source-id: c86f9d0cd1cb0c18de72b06f1bf5847f23f51118
Summary:
For RocksJava 7 we will move from requiring Java 7 to Java 8.
* This simplifies the `Makefile` as we no longer need to deal with Java 7; so we no longer use `javah`.
* Added a java-version target which is invoked by the java target, and which exits if the version of java being used is not 8 or greater.
* Enforces java 8 as a minimum.
* Fixed CMake build.
* Fixed broken java event listener test, as the test was broken and the assertions in the callbacks were not causing assertions in the tests. The callbacks now queue up assertion errors for the main thread of the tests to check.
* Fixed C++ dangling pointers in the test code.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9541
Reviewed By: pdillinger
Differential Revision: D34214929
Pulled By: jay-zhuang
fbshipit-source-id: fdff348758d0a23a742e83c87d5f54073ce16ca6
Summary:
* Fix LIB_MODE=shared for Meta-internal builds (use PIC libraries
appropriately)
* Fix gnu_parallel to recognize CircleCI and Travis builds as not
connected to a terminal (was previously relying on the
`| cat_ignore_eagain` stuff for Ubuntu 16). This problem could cause
timeouts that should be 10m to balloon to 5h.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9553
Test Plan: manual and CI
Reviewed By: jay-zhuang
Differential Revision: D34182886
Pulled By: pdillinger
fbshipit-source-id: e95fd8002d94c8dc414bae1975e4fd348589f2b5
Summary:
Drop support for some old compilers by requiring C++17 standard
(or higher). See https://github.com/facebook/rocksdb/issues/9388
First modification based on this is to remove some conditional compilation in slice.h (also
better for ODR)
Also in this PR:
* Fix some Makefile formatting that seems to affect ASSERT_STATUS_CHECKED config in
some cases
* Add c_test to NON_PARALLEL_TEST in Makefile
* Fix a clang-analyze reported "potential leak" in lru_cache_test
* Better "compatibility" definition of DEFINE_uint32 for old versions of gflags
* Fix a linking problem with shared libraries in Makefile (`./random_test: error while loading shared libraries: librocksdb.so.6.29: cannot open shared object file: No such file or directory`)
* Always set ROCKSDB_SUPPORT_THREAD_LOCAL and use thread_local (from C++11)
* TODO in later PR: clean up that obsolete flag
* Fix a cosmetic typo in c.h (https://github.com/facebook/rocksdb/issues/9488)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9481
Test Plan:
CircleCI config substantially updated.
* Upgrade to latest Ubuntu images for each release
* Generally prefer Ubuntu 20, but keep a couple Ubuntu 16 builds with oldest supported
compilers, to ensure compatibility
* Remove .circleci/cat_ignore_eagain except for Ubuntu 16 builds, because this is to work
around a kernel bug that should not affect anything but Ubuntu 16.
* Remove designated gcc-9 build, because the default linux build now uses GCC 9 from
Ubuntu 20.
* Add some `apt-key add` to fix some apt "couldn't be verified" errors
* Generally drop SKIP_LINK=1; work-around no longer needed
* Generally `add-apt-repository` before `apt-get update` as manual testing indicated the
reverse might not work.
Travis:
* Use gcc-7 by default (remove specific gcc-7 and gcc-4.8 builds)
* TODO in later PR: fix s390x "Assembler messages: Error: invalid switch -march=z14" failure
AppVeyor:
* Completely dropped because we are dropping VS2015 support and CircleCI covers
VS >= 2017
Also local testing with old gflags (out of necessity when using ROCKSDB_NO_FBCODE=1).
Reviewed By: mrambacher
Differential Revision: D33946377
Pulled By: pdillinger
fbshipit-source-id: ae077c823905b45370a26c0103ada119459da6c1
Summary:
**Context/Summary:**
AdvancedColumnFamilyOptions::soft_rate_limit/hard_rate_limit have been marked as deprecated and it's time to actually remove the code.
- Keep `soft_rate_limit`/`hard_rate_limit` in `cf_mutable_options_type_info` to prevent throwing `InvalidArgument` in `GetColumnFamilyOptionsFromMap` when reading an option file still with these options (e.g, old option file generated from RocksDB before the deprecation)
- Keep `soft_rate_limit`/`hard_rate_limit` in under `OptionsOldApiTest.GetOptionsFromMapTest` to test the case mentioned above.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9452
Test Plan: Rely on my eyeball and CI
Reviewed By: ajkr
Differential Revision: D33804938
Pulled By: hx235
fbshipit-source-id: 133d49f7ec5238d7efceeb0a3122a5792a2b9945
Summary:
This PR moves HDFS support from RocksDB repo to a separate repo. The new (temporary?) repo
in this PR serves as an example before we finalize the decision on where and who to host hdfs support. At this point,
people can start from the example repo and fork.
Java/JNI is not included yet, and needs to be done later if necessary.
The goal is to include this commit in RocksDB 7.0 release.
Reference:
https://github.com/ajkr/dedupfs by ajkr
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9170
Test Plan:
Follow the instructions in https://github.com/riversand963/rocksdb-hdfs-env/blob/master/README.md. Build and run db_bench and db_stress.
make check
Reviewed By: ajkr
Differential Revision: D33751662
Pulled By: riversand963
fbshipit-source-id: 22b4db7f31762ed417a20239f5a08dcd1696244f
Summary:
Closing https://github.com/facebook/rocksdb/issues/5954
fsync/fdatasync on Linux:
```
(fsync/fdatasync) includes writing through or flushing a disk cache if present.
```
However, on OS X and iOS:
```
(fsync) will flush all data from the host to the drive (i.e. the "permanent storage device"),
the drive itself may not physically write the data to the platters for quite some time and it
may be written in an out-of-order sequence.
```
Solution is to use `fcntl(F_FULLFSYNC)` on OS X so that we get the same
persistence guarantee.
According to OSX man page,
```
The F_FULLFSYNC fcntl asks the drive to flush **all** buffered data to permanent storage.
```
This suggests that it will be no faster than `fsync` on Linux, since Linux, according to its man page,
```
writing through or flushing a disk cache if present
```
It means Linux may not flush **all** data from disk cache.
This is similar to bug reports/fixes in:
- golang: https://github.com/golang/go/issues/26650
- leveldb: 296de8d5b8.
Not sure if we should fallback to fsync since we break persistence contract.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9356
Reviewed By: jay-zhuang
Differential Revision: D33417416
Pulled By: riversand963
fbshipit-source-id: 475548ff9c5eaccde325e0f6842694271cbc8cb7
Summary:
Replace `-o /dev/null` by `-o test.o` when testing for C++ features such as
-faligned-new otherwise tests will fail with some bugged binutils
(https://sourceware.org/bugzilla/show_bug.cgi?id=19526):
```
output/host/bin/xtensa-buildroot-linux-uclibc-g++ -faligned-new -x c++ - -o /dev/null <<EOF
struct alignas(1024) t {int a;};
int main() {}
EOF
/home/fabrice/buildroot/output/host/lib/gcc/xtensa-buildroot-linux-uclibc/8.3.0/../../../../xtensa-buildroot-linux-uclibc/bin/ld: final link failed: file truncated
```
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6479
Reviewed By: ajkr
Differential Revision: D33574136
Pulled By: riversand963
fbshipit-source-id: 12b48658b17e36013042c98219b89ddf71161d3c
Summary:
As title.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9383
Test Plan:
manually add `using namespace` to a file, and run `make check-sources`.
Then, remove `using namespace`, and run `make check-sources`
Reviewed By: ajkr
Differential Revision: D33551706
Pulled By: riversand963
fbshipit-source-id: 1bb8304f38434da7de0656882e62e77673155725
Summary:
There are some crashes we couldn't debug or repro and couldn't find a core dump. For ASAN the default is `disable_coredump=1` as the doc mentions core dumps can be 16TB+. However I've tried generating them for our `db_stress` commands and they've been in the 1.4-1.6GB range, which is fine. So we can try enabling it in CI.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9330
Test Plan:
- create a test job. (It's internal infra so I put the link in the Phabricator test plan only)
- ran the same command locally, `kill -6 $(pidof db_stress)`, verified core dump showed up
Reviewed By: jay-zhuang
Differential Revision: D33271841
Pulled By: ajkr
fbshipit-source-id: 93b853fa763d5708d078771960ba36854c4be55a
Summary:
More follow-up to https://github.com/facebook/rocksdb/issues/9193 + https://github.com/facebook/rocksdb/issues/9188
* Even though we need to print ETA updates to avoid hitting the 10min
timeout, we need to avoid printing an update if there's no actual
progress, so that hung tests will timeout after 10 min rather than 5
hours.
* When there is a hung test, it's really annoying to track down which
test is hung, so if no progress is observed for 1 minute, we run ps once
to show what is running.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9201
Test Plan: manual and CircleCI
Reviewed By: jay-zhuang
Differential Revision: D32612028
Pulled By: pdillinger
fbshipit-source-id: 00f8ea70fc5fec9ede28ff74287d90fc73854aad
Summary:
Address some issues with https://github.com/facebook/rocksdb/issues/9188
* Internal CI doesn't render \r as anything, so use \n for "not
connected to terminal" case
* CircleCI apparently uses a pseudo-tty for output and although
rerdirect stdout (because of EAGAIN bug) we don't redirect stderr, so it
is detected as a terminal-connected case. Fix by redirecting stderr
also.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9193
Test Plan: manual, CI
Reviewed By: akankshamahajan15
Differential Revision: D32581128
Pulled By: pdillinger
fbshipit-source-id: 5ae7c3209128d8dbd4153c5b9fdb2b810e6deb2e
Summary:
Generating megabytes of successful test output has caused
issues / inconveniences for CI and internal sandcastle runs. This
changes their configuration to only print output from failed tests.
(Successful test output is still available in files under t/.)
This likewise changes default behavior of parallel `make check` as
a quick team poll showed interest in that.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9188
Test Plan:
Seed some test failures and observe
* `make -j24 check` (new behavior)
* `PRINT_PARALLEL_OUTPUTS=1 make -j24 check` (old CI behavior)
* `QUIET_PARALLEL_TESTS=1 make -j24 check` (old manual run behavior)
Reviewed By: siying
Differential Revision: D32567392
Pulled By: pdillinger
fbshipit-source-id: 8d8fb64aebd16bca103b11e3bd1f13c488a69611
Summary:
Move the 'macosx-version-min' arg to the front of PLATFORM_SHARED_LDFLAGS so that it doesn't get concatenated with the library name. Fixes https://github.com/facebook/rocksdb/issues/9146
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9149
Reviewed By: mrambacher
Differential Revision: D32396101
Pulled By: pdillinger
fbshipit-source-id: aefcf53384e64d399049f158779acc3a4e54a8fe
Summary:
Force POSIX locale for calls to 'git remote' that might have
locale-dependent formatting, as shown in https://github.com/facebook/rocksdb/issues/8731 comment
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9079
Test Plan:
manual (haven't tried on a machine with non-english default
locale)
Reviewed By: ltamasi
Differential Revision: D31943092
Pulled By: pdillinger
fbshipit-source-id: 7dbe5915824f39f73b412cc3d1a86a2521cf76c1
Summary:
This PR adds support for building on s390x including updating travis CI. It uses the previous work in https://github.com/facebook/rocksdb/pull/6168 and adds some more changes to get all current tests (make check and jni tests) to pass. The tests were run with snappy, lz4, bzip2 and zstd all compiled in.
There are a few pieces still needed to get the travis build working that I don't think I can do. adamretter is this something you could help with?
1. A prebuilt https://rocksdb-deps.s3-us-west-2.amazonaws.com/cmake/cmake-3.14.5-Linux-s390x.deb package
2. A https://hub.docker.com/r/evolvedbinary/rocksjava s390x image
Not sure if there is more required for travis. Happy to help in any way I can.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8962
Reviewed By: mrambacher
Differential Revision: D31802198
Pulled By: pdillinger
fbshipit-source-id: 683511466fa6b505f85ba5a9964a268c6151f0c2
Summary:
I get `clang-format-diff` after running `apt install clang-format` on Ubuntu instead of `clang-format-diff.py`. So I think it makes sense to make the format script compatible with this behavior.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/9028
Reviewed By: ajkr
Differential Revision: D31634041
Pulled By: jay-zhuang
fbshipit-source-id: b936de791ddcafa6ff304039ef33936e1e04864d
Summary:
Instead of hardcoding the stress test type and some args, allow it to be passed through env variables.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8985
Reviewed By: akankshamahajan15
Differential Revision: D31349495
Pulled By: anand1976
fbshipit-source-id: 585c8fcb0232d0a95925b1a8c4e42a0940227e8b
Summary:
For internal build enviroment only. Developer could run the
microbenchmark without `ROCKSDB_NO_FBCODE=1`.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8954
Test Plan: `$ make microbench` on dev server
Reviewed By: ajkr
Differential Revision: D31163717
Pulled By: jay-zhuang
fbshipit-source-id: 1ff59f660ca05afd0fd5c7c7dcdfd831ac365462
Summary:
One contrun name is incorrect, which mixed error reporting with another one. Fix it.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8924
Reviewed By: ltamasi
Differential Revision: D30999477
fbshipit-source-id: 46a04b2e4b48f755181aa9a47c353d91f1128469
Summary:
* Don't hardcode namespace rocksdb (use ROCKSDB_NAMESPACE)
* Don't #include <rocksdb/...> (use double quotes)
* Support putting NOCOMMIT (any case) in source code that should not be
committed/pushed in current state.
These will be run with `make check` and in GitHub actions
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8821
Test Plan: existing tests, manually try out new checks
Reviewed By: zhichao-cao
Differential Revision: D30791726
Pulled By: pdillinger
fbshipit-source-id: 399c883f312be24d9e55c58951d4013e18429d92
Summary:
asan_symbolize.py is not compatible with python3. Also make it
consistent with public CI, which doesn't use asan_symbolize.py
And update coverage_test.sh to use python3.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8737
Test Plan: CI
Reviewed By: pdillinger
Differential Revision: D30702430
Pulled By: jay-zhuang
fbshipit-source-id: ef09947b1232294d31b09a855c2f0ce149097dd9
Summary:
Regression test is broken and not running:
1. failed test is not reporting, fix it by add `set -e`
2. internal regression test is not run inside github, removing that
3. fix a few minor issues to pass the test
4. delete unused binary size build, and regression test is reporting binary size now.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8753
Test Plan: CI: https://www.internalfb.com/intern/sandcastle/job/13510799359573861
Reviewed By: ltamasi
Differential Revision: D30754380
Pulled By: jay-zhuang
fbshipit-source-id: 0cfa008327fff31bc61118a3fe642924090d28e1
Summary:
Required for platform009, which is incompat with the centos
assembler.
author: pbrady@fb.com D29099768
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8733
Test Plan: CI
Reviewed By: pixelb
Differential Revision: D30687156
Pulled By: jay-zhuang
fbshipit-source-id: 796f90842cbf0ca11bad07e7d654bce1fafc4ba0
Summary:
Allow extra arguments to be passed to db_stress in fbcode crash tests by the ```rocksdb-lego-determinator``` invoker.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8587
Reviewed By: zhichao-cao
Differential Revision: D29940217
Pulled By: anand1976
fbshipit-source-id: 17cbcd2def60eff2a895553f917694496c4742aa
Summary:
DistributedMutex hasn't been used in the code base and enabling
`USE_FOLLY_DISTRIBUTED_MUTEX` only runs the mutex tests from third-party
lib. So disabling it for now.
The implementation may also out of date, should re-sync with folly before
using.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8584
Test Plan: CI
Reviewed By: ajkr
Differential Revision: D29888960
Pulled By: jay-zhuang
fbshipit-source-id: 3e75f73386c6ed03efb96a1400258d602a724f17
Summary:
Add google benchmark for microbench.
Add ribbon_bench for benchmark ribbon filter vs. other filters.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8493
Test Plan:
added test to CI
To run the benchmark on devhost:
Install benchmark: `$ sudo dnf install google-benchmark-devel`
Build and run:
`$ ROCKSDB_NO_FBCODE=1 DEBUG_LEVEL=0 make microbench`
or with cmake:
`$ mkdir build && cd build && cmake .. -DCMAKE_BUILD_TYPE=Release -DWITH_BENCHMARK=1 && make microbench`
Reviewed By: pdillinger
Differential Revision: D29589649
Pulled By: jay-zhuang
fbshipit-source-id: 8fed13b562bef4472f161ecacec1ab6b18911dff
Summary:
Add a new test ```fbcode_crash_test``` to rocksdb-lego-determinator. This test allows the crash test to be run on Facebook Sandcastle infra using fbcode components. Also use the default Env in db_stress to access the expected values path as it requires a memory mapped file and may not work with custom Envs.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8471
Reviewed By: ajkr
Differential Revision: D29474722
Pulled By: anand1976
fbshipit-source-id: 7d086d82dd7091ae48e08cb4ace763ce3e3b87ef
