Fix compact_files_example

Summary:
See task #7983654. The example was triggering an assert in compaction job
because the compaction was not marked as manual. With this patch,
CompactionPicker::FormCompaction() marks compactions as manual. This patch
also fixes a couple of typos, adds optimistic_transaction_example to
.gitignore and librocksdb as a dependency for examples. Adding librocksdb as
a dependency makes sure that the examples are built with the latest changes
in librocksdb.

Test Plan: make clean && cd examples && make all && ./compact_files_example

Reviewers: rven, sdong, anthony, igor, yhchiang

Reviewed By: yhchiang

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D45117

db/compaction_picker.cc

@@ -261,10 +261,10 @@ Compaction* CompactionPicker::FormCompaction(
           mutable_cf_options.MaxGrandParentOverlapBytes(output_level + 1) :
           std::numeric_limits<uint64_t>::max();
   assert(input_files.size());
-  return new Compaction(vstorage, mutable_cf_options, input_files, output_level,
+  return new Compaction(
-                        compact_options.output_file_size_limit,
+      vstorage, mutable_cf_options, input_files, output_level,
-                        max_grandparent_overlap_bytes, output_path_id,
+      compact_options.output_file_size_limit, max_grandparent_overlap_bytes,
-                        compact_options.compression, /* grandparents */ {});
+      output_path_id, compact_options.compression, /* grandparents */ {}, true);
 }
 
 Status CompactionPicker::GetCompactionInputsFromFileNumbers(

db/compaction_picker.h

@@ -8,22 +8,20 @@
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
 #pragma once
-#include <vector>
-#include <memory>
-#include <set>
-#include <unordered_set>
 
-#include "db/version_set.h"
-#include "db/compaction.h"
-#include "rocksdb/status.h"
-#include "rocksdb/options.h"
-#include "rocksdb/env.h"
-#include "util/mutable_cf_options.h"
-
-#include <vector>
 #include <memory>
 #include <set>
 #include <string>
+#include <unordered_set>
+#include <vector>
+
+#include "db/compaction.h"
+#include "db/version_set.h"
+#include "rocksdb/env.h"
+#include "rocksdb/options.h"
+#include "rocksdb/status.h"
+#include "util/mutable_cf_options.h"
+
 
 namespace rocksdb {
 
@@ -90,7 +88,8 @@ class CompactionPicker {
   // Returns true if any one of the specified files are being compacted
   bool FilesInCompaction(const std::vector<FileMetaData*>& files);
 
-  // Takes a list of CompactionInputFiles and returns a Compaction object.
+  // Takes a list of CompactionInputFiles and returns a (manual) Compaction
+  // object.
   Compaction* FormCompaction(
       const CompactionOptions& compact_options,
       const std::vector<CompactionInputFiles>& input_files, int output_level,

db/db_impl.cc

@@ -1469,7 +1469,7 @@ Status DBImpl::CompactRange(const CompactRangeOptions& options,
   } else {
     for (int level = 0; level <= max_level_with_files; level++) {
       int output_level;
-      // in case the compaction is unversal or if we're compacting the
+      // in case the compaction is universal or if we're compacting the
       // bottom-most level, the output level will be the same as input one.
       // level 0 can never be the bottommost level (i.e. if all files are in
       // level 0, we will compact to level 1)
@@ -1485,8 +1485,8 @@ Status DBImpl::CompactRange(const CompactRangeOptions& options,
                        BottommostLevelCompaction::kIfHaveCompactionFilter &&
                    cfd->ioptions()->compaction_filter == nullptr &&
                    cfd->ioptions()->compaction_filter_factory == nullptr) {
-          // Skip bottommost level compaction since we dont have
+          // Skip bottommost level compaction since we don't have a compaction
-          // compaction filter
+          // filter
           continue;
         }
         output_level = level;

db/db_universal_compaction_test.cc

@@ -335,9 +335,10 @@ TEST_P(DBTestUniversalCompaction, CompactFilesOnUniversalCompaction) {
   }
 
   // expect fail since universal compaction only allow L0 output
-  ASSERT_TRUE(!dbfull()->CompactFiles(
+  ASSERT_FALSE(dbfull()
-      CompactionOptions(), handles_[1],
+                   ->CompactFiles(CompactionOptions(), handles_[1],
-      compaction_input_file_names, 1).ok());
+                                  compaction_input_file_names, 1)
+                   .ok());
 
   // expect ok and verify the compacted files no longer exist.
   ASSERT_OK(dbfull()->CompactFiles(

examples/.gitignore

@@ -3,3 +3,4 @@ simple_example
 c_simple_example
 compact_files_example
 transaction_example
+optimistic_transaction_example

examples/Makefile

@@ -1,29 +1,32 @@
 include ../make_config.mk
 
-.PHONY: clean
+.PHONY: clean librocksdb
 
 all: simple_example column_families_example compact_files_example c_simple_example optimistic_transaction_example transaction_example
 
-simple_example: simple_example.cc
+simple_example: librocksdb simple_example.cc
 	$(CXX) $(CXXFLAGS) $@.cc -o$@ ../librocksdb.a -I../include -O2 -std=c++11 $(PLATFORM_LDFLAGS) $(PLATFORM_CXXFLAGS) $(EXEC_LDFLAGS)
 
-column_families_example: column_families_example.cc
+column_families_example: librocksdb column_families_example.cc
 	$(CXX) $(CXXFLAGS) $@.cc -o$@ ../librocksdb.a -I../include -O2 -std=c++11 $(PLATFORM_LDFLAGS) $(PLATFORM_CXXFLAGS) $(EXEC_LDFLAGS)
 
-compact_files_example: compact_files_example.cc
+compact_files_example: librocksdb compact_files_example.cc
 	$(CXX) $(CXXFLAGS) $@.cc -o$@ ../librocksdb.a -I../include -O2 -std=c++11 $(PLATFORM_LDFLAGS) $(PLATFORM_CXXFLAGS) $(EXEC_LDFLAGS)
 
 .c.o:
 	$(CC) $(CFLAGS) -c $< -o $@ -I../include
 
-c_simple_example: c_simple_example.o
+c_simple_example: librocksdb c_simple_example.o
 	$(CXX) $@.o -o$@ ../librocksdb.a $(PLATFORM_LDFLAGS) $(EXEC_LDFLAGS)
 
-optimistic_transaction_example: optimistic_transaction_example.cc
+optimistic_transaction_example: librocksdb optimistic_transaction_example.cc
 	$(CXX) $(CXXFLAGS) $@.cc -o$@ ../librocksdb.a -I../include -O2 -std=c++11 $(PLATFORM_LDFLAGS) $(PLATFORM_CXXFLAGS) $(EXEC_LDFLAGS)
 
-transaction_example: transaction_example.cc
+transaction_example: librocksdb transaction_example.cc
 	$(CXX) $(CXXFLAGS) $@.cc -o$@ ../librocksdb.a -I../include -O2 -std=c++11 $(PLATFORM_LDFLAGS) $(PLATFORM_CXXFLAGS) $(EXEC_LDFLAGS)
 
 clean:
 	rm -rf ./simple_example ./column_families_example ./compact_files_example ./c_simple_example c_simple_example.o ./optimistic_transaction_example ./transaction_example
+
+librocksdb:
+	cd .. && $(MAKE) librocksdb.a

examples/compact_files_example.cc

@@ -67,9 +67,9 @@ class FullCompactor : public Compactor {
         options_.target_file_size_base;
   }
 
-  // When flush happens, it determins whether to trigger compaction.
+  // When flush happens, it determines whether to trigger compaction. If
-  // If triggered_writes_stop is true, it will also set the retry
+  // triggered_writes_stop is true, it will also set the retry flag of
-  // flag of compaction-task to true.
+  // compaction-task to true.
   void OnFlushCompleted(
       DB* db, const FlushJobInfo& info) override {
     CompactionTask* task = PickCompaction(db, info.cf_name);
@@ -108,7 +108,8 @@ class FullCompactor : public Compactor {
   }
 
   static void CompactFiles(void* arg) {
-    CompactionTask* task = reinterpret_cast<CompactionTask*>(arg);
+    std::unique_ptr<CompactionTask> task(
+        reinterpret_cast<CompactionTask*>(arg));
     assert(task);
     assert(task->db);
     Status s = task->db->CompactFiles(
@@ -124,8 +125,6 @@ class FullCompactor : public Compactor {
           task->db, task->column_family_name);
       task->compactor->ScheduleCompaction(new_task);
     }
-    // release the task
-    delete task;
   }
 
  private:

include/rocksdb/db.h

@@ -489,10 +489,10 @@ class DB {
     return SetOptions(DefaultColumnFamily(), new_options);
   }
 
-  // CompactFiles() inputs a list of files specified by file numbers
+  // CompactFiles() inputs a list of files specified by file numbers and
-  // and compacts them to the specified level.  Note that the behavior
+  // compacts them to the specified level. Note that the behavior is different
-  // is different from CompactRange in that CompactFiles() will
+  // from CompactRange() in that CompactFiles() performs the compaction job
-  // perform the compaction job using the CURRENT thread.
+  // using the CURRENT thread.
   //
   // @see GetDataBaseMetaData
   // @see GetColumnFamilyMetaData

util/db_test_util.cc

@@ -188,7 +188,7 @@ Options DBTestBase::CurrentOptions(
 Options DBTestBase::CurrentOptions(
     const Options& defaultOptions,
     const anon::OptionsOverride& options_override) {
-  // this redudant copy is to minimize code change w/o having lint error.
+  // this redundant copy is to minimize code change w/o having lint error.
   Options options = defaultOptions;
   XFUNC_TEST("", "dbtest_options", inplace_options1, GetXFTestOptions,
              reinterpret_cast<Options*>(&options),