Skip to content

Commit

Permalink
Fix FuzzSimple and FuzzComplex VeloxReaderTests (#117)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #117

With the recent change in D66741079, WriterOptions keeps a keep alive
to the executor, what prevents an executor from being destructed. Fixing the
destruction order of these variables to prevent this situation in these two
tests.

Reviewed By: darrenfu

Differential Revision: D67374064

fbshipit-source-id: e901516c3a3a98299267bd65879b0192867ddbe2
  • Loading branch information
pedroerp authored and facebook-github-bot committed Dec 18, 2024
1 parent 68dd6ff commit 3b70f92
Showing 1 changed file with 14 additions and 7 deletions.
21 changes: 14 additions & 7 deletions dwio/nimble/velox/tests/VeloxReaderTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2429,8 +2429,11 @@ TEST_F(VeloxReaderTests, FuzzSimple) {

for (auto parallelismFactor : {0U, 1U, std::thread::hardware_concurrency()}) {
LOG(INFO) << "Parallelism Factor: " << parallelismFactor;
nimble::VeloxWriterOptions writerOptions;

// Executor needs to outlive writerOptions since the latter has a keepAlive
// on the former.
std::shared_ptr<folly::CPUThreadPoolExecutor> executor;
nimble::VeloxWriterOptions writerOptions;

if (parallelismFactor > 0) {
executor =
Expand Down Expand Up @@ -2493,11 +2496,6 @@ TEST_F(VeloxReaderTests, FuzzComplex) {
: folly::Random::rand32();
LOG(INFO) << "seed: " << seed;

nimble::VeloxWriterOptions writerOptions;
writerOptions.dictionaryArrayColumns.insert("nested_map_array1");
writerOptions.dictionaryArrayColumns.insert("nested_map_array2");
writerOptions.dictionaryArrayColumns.insert("dict_array");
writerOptions.deduplicatedMapColumns.insert("dict_map");
// Small batches creates more edge cases.
size_t batchSize = 10;
velox::VectorFuzzer noNulls(
Expand Down Expand Up @@ -2528,9 +2526,18 @@ TEST_F(VeloxReaderTests, FuzzComplex) {
auto batches = 20;
std::mt19937 rng{seed};

// Executor needs to outlive writerOptions since the latter has a keepAlive on
// the former.
std::shared_ptr<folly::CPUThreadPoolExecutor> executor;

for (auto parallelismFactor : {0U, 1U, std::thread::hardware_concurrency()}) {
LOG(INFO) << "Parallelism Factor: " << parallelismFactor;
std::shared_ptr<folly::CPUThreadPoolExecutor> executor;

nimble::VeloxWriterOptions writerOptions;
writerOptions.dictionaryArrayColumns.insert("nested_map_array1");
writerOptions.dictionaryArrayColumns.insert("nested_map_array2");
writerOptions.dictionaryArrayColumns.insert("dict_array");
writerOptions.deduplicatedMapColumns.insert("dict_map");

if (parallelismFactor > 0) {
executor =
Expand Down

0 comments on commit 3b70f92

Please sign in to comment.