issue #934: multi-thread compaction support #976

caijieming-ng · 2016-08-18T13:28:01Z

#934 compact优化，story3，多线程compact

caijieming-ng · 2016-08-18T13:28:44Z

build

baidubot · 2016-08-18T13:29:08Z

taocp · 2016-09-06T03:35:29Z

src/leveldb/db/db_impl.cc

+  // check filesystem, and then check pending_outputs_
+  std::vector<std::string> filenames;
+  mutex_.Unlock();
+  env_->GetChildren(dbname_, &filenames); // Ignoring errors on purpose


why Ignoring errors on purpose

即使本次删除旧sst失败，下一次compact也会继续删除，所以没有必要因为此处失败，而认为本次compact失败，compact重做的话，io开销很重。

taocp · 2016-09-08T02:23:13Z

src/leveldb/db/db_table.cc

@@ -219,10 +219,10 @@ Status DBTable::Init() {
    std::vector<uint64_t> snapshot_sequence = options_.snapshots_sequence;
    std::map<uint64_t, uint64_t> rollbacks = options_.rollbacks;
    for (std::set<uint32_t>::iterator it = options_.exist_lg_list->begin();
-         it != options_.exist_lg_list->end() && s.ok(); ++it) {
+            it != options_.exist_lg_list->end() && s.ok(); ++it) {


改这个缩进是基于一种怎样的考虑？

taocp · 2016-09-08T05:04:56Z

@baidu/tera_dev 请都瞧一瞧。

LGTM

00k · 2016-09-13T07:05:49Z

build

lylei · 2016-09-19T03:02:11Z

build

yvxiang · 2016-10-13T07:53:45Z

src/leveldb/db/db_impl.h

    const InternalKey* begin;   // NULL means beginning of key range
    const InternalKey* end;     // NULL means end of key range
    InternalKey tmp_storage;    // Used to keep track of compaction progress
+    int compaction_conflict; // 0 == idle, 1 == conflict, 2 == wake


直接用ManualCompactState?

enum内部也是个int吧

只是感觉这样好读

00k · 2016-10-26T04:54:08Z

commit log里，multithread中间加个-，compactiong拼错了

caijieming-ng · 2016-11-21T08:20:23Z

已修改

caijieming-ng · 2016-11-23T10:36:37Z

build

caijieming-ng · 2016-11-23T11:32:09Z

build

caijieming-ng · 2016-12-01T08:23:39Z

build

caijieming-ng · 2016-12-01T08:30:20Z

build

taocp · 2017-05-11T02:20:07Z

在看ing，需要点时间。。稍安勿躁！

taocp

求补点注释：）

taocp · 2017-04-18T12:27:57Z

src/leveldb/db/db_impl.cc

+  }
+  imm_->SetBeingFlushed(true);
+
+  if (imm_->ApproximateMemoryUsage() <= 0) { // imm is empty, do nothing


这是什么情况？

代码完整性检查：memtablet没有数据时，不做内存的dump

taocp · 2017-05-12T03:25:51Z

src/leveldb/db/db_impl.h

+  struct CompactionTask {
+    int64_t id;
+    double score;
+    uint64_t timeout;


求注释下各个字段的基本含义

taocp · 2017-05-12T03:28:16Z

src/leveldb/db/db_impl.h

@@ -110,19 +116,19 @@ class DBImpl : public DB {

  // Compact the in-memory write buffer to disk.  Switches to a new
  // log-file/memtable and writes a new descriptor iff successful.
-  Status CompactMemTable()
+  Status CompactMemTable(bool* sched_idle = NULL)


函数的参数求注释啊，下同

描述改函数是否真正干活，避免空调度

防止空调度

taocp · 2017-05-15T05:13:16Z

src/leveldb/db/db_impl.cc

 // end of tera-specific

+bool ScoreSortDesc(std::pair<double, uint64_t> i, std::pair<double, uint64_t> j) {


函数名求不要简写，看不懂。。。

taocp · 2017-05-15T06:26:46Z

src/leveldb/db/db_impl.cc

    c->ReleaseInputs();
-    DeleteObsoleteFiles();
+    DeleteObsoleteFiles();// delete any sst not in version set. if comapct and flush mem handle in multi-thread, may delete new data


这个注释啥意思？

taocp · 2017-05-15T11:06:59Z

src/leveldb/db/db_impl.h


  // Information for a manual compaction
+  enum ManualCompactState {


跪求注释，这几种状态的定义

elithnever

缺少单测吧?

elithnever · 2017-05-26T06:15:45Z

src/leveldb/db/db_impl.h

-  uint64_t bg_compaction_timeout_;
-  int64_t bg_schedule_id_;
+  std::vector<CompactionTask*> bg_compaction_tasks_;
+  int bg_compaction_scheduled_;


bg_compaction_scheduled_不需要了吧? bg_compaction_score_可以计算出来?

elithnever · 2017-05-26T06:17:19Z

src/leveldb/db/db_impl.h

+  std::vector<CompactionTask*> bg_compaction_tasks_;
+  int bg_compaction_scheduled_;
+  std::vector<double> bg_compaction_score_;
+  std::vector<int64_t> bg_schedule_id_;


bg_compaction_score_和bg_schedule_id_ 放在一个结构体里?

score和id不是同时产生的，没有必要放到同一个结构体里面

elithnever · 2017-05-26T06:20:24Z

src/leveldb/db/db_impl.h

    const InternalKey* begin;   // NULL means beginning of key range
    const InternalKey* end;     // NULL means end of key range
    InternalKey tmp_storage;    // Used to keep track of compaction progress
+    ManualCompactState compaction_conflict; // 0 == idle, 1 == conflict, 2 == wake


compaction_conflict变量命名有点奇怪? 猜不出什么含义?

就是手动compact并发冲突

elithnever · 2017-05-26T06:27:58Z

src/leveldb/db/memtable.h

@@ -79,6 +79,13 @@ class MemTable {
    empty_ = false;
  }

+  bool BeingFlushed() { return being_flushed_;}
+  void SetBeingFlushed(bool flag) {
+    assert(flag ? !being_flushed_


为什么要加assert?

放置后续同学改次块代码改错

elithnever · 2017-05-26T07:03:00Z

src/leveldb/include/leveldb/options.h

@@ -314,6 +314,14 @@ struct Options {
  // Default: 10 %
  uint64_t del_percentage;


注释中的default值是10, 然而实际的default值是20

elithnever · 2017-05-26T08:26:45Z

src/leveldb/db/db_impl.cc

@@ -746,13 +772,19 @@ Status DBImpl::CompactMemTable() {
    Log(options_.info_log, "[%s] CompactMemTable SetLastSequence %lu",
        dbname_.c_str(), edit.GetLastSequence());
    s = versions_->LogAndApply(&edit, &mutex_);
+    pending_outputs_.erase(number);
+  } else {
+    // TODO: dump memtable error, delete temp file


把TODO补上逻辑还是不需要TODO?

elithnever · 2017-05-26T08:28:43Z

src/leveldb/db/db_impl.cc

+  MutexLock lock(&mutex_);
+  Status s;
+  if (recover_mem_ == NULL) {
+      DeleteObsoleteFiles();


recover阶段需要增加这步吗?

elithnever · 2017-05-26T08:33:38Z

src/leveldb/db/db_impl.cc

  }

-  bg_compaction_scheduled_ = false;
+  bg_compaction_scheduled_--;
+  std::vector<CompactionTask*>::iterator task_id = std::find(bg_compaction_tasks_.begin(),


是代码格式乱了, 还是显示的乱了?

显示问题

elithnever · 2017-05-26T08:34:48Z

src/leveldb/db/db_impl.cc

    c->ReleaseInputs();
-    DeleteObsoleteFiles();
+    DeleteObsoleteFiles();// delete any sst not in version set. if comapct and flush mem handle in multi-thread, may delete new data


elithnever · 2017-05-26T08:36:29Z

src/leveldb/db/db_impl.cc

@@ -1118,6 +1216,7 @@ void DBImpl::CleanupCompaction(CompactionState* compact) {
  for (size_t i = 0; i < compact->outputs.size(); i++) {
    const CompactionState::Output& out = compact->outputs[i];
    pending_outputs_.erase(BuildFullFileNumber(dbname_, out.number));
+    Log(options_.info_log, "[%s] erase pending_output #%lu", dbname_.c_str(), out.number);


日志里最好都写filename, 便于追查问题, 另外这个日志会不会输出量较大?

追查问题就是用sst number

caijieming-ng · 2017-06-07T03:19:02Z

db_test里面基本能覆盖多线程compact的测试

taocp · 2017-06-07T07:02:22Z

LGTM

baidubot assigned taocp Aug 18, 2016

caijieming-ng force-pushed the compact_multithread branch 2 times, most recently from 9c789c6 to b38af4b Compare August 25, 2016 02:40

caijieming-ng mentioned this pull request Aug 25, 2016

Mc doc #985

Open

caijieming-ng force-pushed the compact_multithread branch 3 times, most recently from 4ee16ed to d84e0b7 Compare August 30, 2016 04:43

taocp reviewed Sep 6, 2016
View reviewed changes

caijieming-ng force-pushed the compact_multithread branch 3 times, most recently from f28504a to e61e91b Compare September 7, 2016 18:31

taocp reviewed Sep 8, 2016
View reviewed changes

caijieming-ng force-pushed the compact_multithread branch 2 times, most recently from 59b896b to 1741aa5 Compare September 8, 2016 04:46

caijieming-ng force-pushed the compact_multithread branch from 1741aa5 to bc1ff22 Compare September 12, 2016 15:45

caijieming-ng force-pushed the compact_multithread branch from bc1ff22 to 8585854 Compare October 10, 2016 07:18

yvxiang reviewed Oct 13, 2016

View reviewed changes

caijieming-ng force-pushed the compact_multithread branch 2 times, most recently from 5ec0c93 to 549443c Compare November 21, 2016 08:20

caijieming-ng changed the title ~~issue #934: multithread compactiong support~~ issue #934: multi-thread compaction support Nov 21, 2016

caijieming-ng force-pushed the compact_multithread branch from 549443c to 58cb9cf Compare November 22, 2016 13:17

caijieming-ng force-pushed the compact_multithread branch from 58cb9cf to 691adcf Compare November 23, 2016 11:30

caijieming-ng force-pushed the compact_multithread branch from 691adcf to 0dc050b Compare April 18, 2017 02:33

taocp reviewed May 15, 2017

View reviewed changes

elithnever reviewed May 26, 2017

View reviewed changes

issue baidu#934: multi-thread compaction support

b45434e

caijieming-ng force-pushed the compact_multithread branch from 17702ce to b45434e Compare July 5, 2017 16:34

		// end of tera-specific

		bool ScoreSortDesc(std::pair<double, uint64_t> i, std::pair<double, uint64_t> j) {


		// Information for a manual compaction
		enum ManualCompactState {

		@@ -314,6 +314,14 @@ struct Options {
		// Default: 10 %
		uint64_t del_percentage;

issue #934: multi-thread compaction support #976

Are you sure you want to change the base?

issue #934: multi-thread compaction support #976

Conversation

caijieming-ng commented Aug 18, 2016

caijieming-ng commented Aug 18, 2016

baidubot commented Aug 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taocp commented Sep 8, 2016 • edited

00k commented Sep 13, 2016

lylei commented Sep 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

00k commented Oct 26, 2016

caijieming-ng commented Nov 21, 2016

caijieming-ng commented Nov 23, 2016

caijieming-ng commented Nov 23, 2016

caijieming-ng commented Dec 1, 2016

caijieming-ng commented Dec 1, 2016

taocp commented May 11, 2017

taocp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caijieming-ng Jun 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elithnever left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caijieming-ng Jun 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caijieming-ng commented Jun 7, 2017

taocp commented Jun 7, 2017

taocp commented Sep 8, 2016 •

edited

caijieming-ng Jun 7, 2017 •

edited

caijieming-ng Jun 7, 2017 •

edited