issue #934: multi-thread compaction support by caijieming-ng · Pull Request #976 · baidu/tera

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

    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

  // 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

+    versions_->ReleaseCompaction(c, status); // current_version has reference to c->inputs_[0,1]
    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

+  std::vector<int64_t> bg_schedule_id_;

  // 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


+  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

    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

+    versions_->ReleaseCompaction(c, status); // current_version has reference to c->inputs_[0,1]
    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

  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) {

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

Conversation

caijieming-ng commented Aug 18, 2016

Uh oh!

caijieming-ng commented Aug 18, 2016

Uh oh!

baidubot commented Aug 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taocp commented Sep 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

00k commented Sep 13, 2016

Uh oh!

lylei commented Sep 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

00k commented Oct 26, 2016

Uh oh!

caijieming-ng commented Nov 21, 2016

Uh oh!

caijieming-ng commented Nov 23, 2016

Uh oh!

caijieming-ng commented Nov 23, 2016

Uh oh!

caijieming-ng commented Dec 1, 2016

Uh oh!

caijieming-ng commented Dec 1, 2016

Uh oh!

taocp commented May 11, 2017

Uh oh!

taocp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caijieming-ng Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elithnever left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taocp commented Sep 8, 2016 •

edited

Loading

caijieming-ng Jun 7, 2017 •

edited

Loading

caijieming-ng Jun 7, 2017 •

edited

Loading