From 1d46ed7454dff23610da792e5d3ed31b8add58d6 Mon Sep 17 00:00:00 2001
From: yuuya uezato <yuuya_uezato@dwango.co.jp>
Date: Fri, 10 May 2019 16:48:18 +0900
Subject: [PATCH 1/4] =?UTF-8?q?issue28=E3=81=AE=E3=81=9F=E3=82=81=E3=81=AE?=
 =?UTF-8?q?=E6=96=B0=E3=81=97=E3=81=84=E3=83=91=E3=83=83=E3=83=81?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/storage/builder.rs             |  8 +++++++
 src/storage/data_region.rs         | 31 ++++++++++++++++++++++++-
 src/storage/index.rs               |  2 +-
 src/storage/journal/nvm_buffer.rs  |  4 ++++
 src/storage/journal/region.rs      |  9 +++++++-
 src/storage/journal/ring_buffer.rs |  4 ++++
 src/storage/mod.rs                 | 37 ++++++++++++++++++++++++++----
 7 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/src/storage/builder.rs b/src/storage/builder.rs
index 25746e3..e8947e3 100644
--- a/src/storage/builder.rs
+++ b/src/storage/builder.rs
@@ -23,6 +23,7 @@ pub struct StorageBuilder {
     instance_uuid: Option<Uuid>,
     journal: JournalRegionOptions,
     metrics: MetricBuilder,
+    safe_release_mode: bool,
 }
 impl StorageBuilder {
     /// 新しい`StorageBuilder`インスタンスを生成する.
@@ -32,9 +33,16 @@ impl StorageBuilder {
             instance_uuid: None,
             journal: JournalRegionOptions::default(),
             metrics: MetricBuilder::new(),
+            safe_release_mode: false,
         }
     }
 
+    /// [issue28](https://github.com/frugalos/cannyls/issue28)
+    pub fn enable_safe_release_mode(&mut self) -> &mut Self {
+        self.safe_release_mode = true;
+        self
+    }
+
     /// ストレージインスタンスを識別するためのUUIDを設定する.
     ///
     /// ストレージの作成時とオープン時で、指定した値の使われ方が異なる:
diff --git a/src/storage/data_region.rs b/src/storage/data_region.rs
index 8cf3752..7ae2cf3 100644
--- a/src/storage/data_region.rs
+++ b/src/storage/data_region.rs
@@ -19,6 +19,8 @@ pub struct DataRegion<N> {
     nvm: N,
     block_size: BlockSize,
     metrics: DataRegionMetrics,
+    safe_release_mode: bool,
+    reserved_portions: Vec<DataPortion>,
 }
 impl<N> DataRegion<N>
 where
@@ -34,9 +36,24 @@ where
             nvm,
             block_size,
             metrics: DataRegionMetrics::new(metric_builder, capacity, allocator_metrics),
+            safe_release_mode: false,
+            reserved_portions: Vec::new(),
         }
     }
 
+    /// 安全にリソースを解放するモードに移行するためのメソッド。
+    /// * `true`を渡すと、安全な解放モードに入る。
+    ///    * 安全な解放については [issue28](https://github.com/frugalos/cannyls/issue28) を参考のこと。
+    /// * `false`を渡すと、削除されたデータポーションが即座にアロケータから解放される。
+    pub fn enable_safe_release_mode(&mut self, enabling: bool) {
+        if !enabling && !self.reserved_portions.is_empty() {
+            // 削除対象ポーションが存在する状況で即時削除モードに切り替える場合は
+            // この段階で削除を行う
+            self.release_pending_portions();
+        }
+        self.safe_release_mode = enabling;
+    }
+
     /// データ領域のメトリクスを返す.
     pub fn metrics(&self) -> &DataRegionMetrics {
         &self.metrics
@@ -89,7 +106,19 @@ where
     /// `portion`で未割当の領域が指定された場合には、
     /// 現在の実行スレッドがパニックする.
     pub fn delete(&mut self, portion: DataPortion) {
-        self.allocator.release(portion);
+        if self.safe_release_mode {
+            self.reserved_portions.push(portion);
+        } else {
+            self.allocator.release(portion);
+        }
+    }
+
+    pub fn release_pending_portions(&mut self) {
+        if self.safe_release_mode {
+            for p in ::std::mem::replace(&mut self.reserved_portions, Vec::new()) {
+                self.allocator.release(p);
+            }
+        }
     }
 
     /// 部分領域の単位をブロックからバイトに変換する.
diff --git a/src/storage/index.rs b/src/storage/index.rs
index 05cbae9..8e77e31 100644
--- a/src/storage/index.rs
+++ b/src/storage/index.rs
@@ -38,7 +38,7 @@ impl LumpIndex {
     ///
     /// 結果は昇順にソートされている.
     pub fn remove(&mut self, lump_id: &LumpId) -> Option<Portion> {
-        self.map.remove(lump_id).map(|p| p.into())
+        self.map.remove(lump_id).map(std::convert::Into::into)
     }
 
     /// 登録されているlumpのID一覧を返す.
diff --git a/src/storage/journal/nvm_buffer.rs b/src/storage/journal/nvm_buffer.rs
index 0b699a2..2533fa9 100644
--- a/src/storage/journal/nvm_buffer.rs
+++ b/src/storage/journal/nvm_buffer.rs
@@ -82,6 +82,10 @@ impl<N: NonVolatileMemory> JournalNvmBuffer<N> {
         &self.inner
     }
 
+    pub fn is_dirty(&self) -> bool {
+        self.maybe_dirty
+    }
+
     fn is_dirty_area(&self, offset: u64, length: usize) -> bool {
         if !self.maybe_dirty || length == 0 || self.write_buf.is_empty() {
             return false;
diff --git a/src/storage/journal/region.rs b/src/storage/journal/region.rs
index fd179e3..3a641b4 100644
--- a/src/storage/journal/region.rs
+++ b/src/storage/journal/region.rs
@@ -152,6 +152,7 @@ where
         if self.gc_queue.is_empty() {
             track!(self.fill_gc_queue())?;
         } else if self.sync_countdown != self.options.sync_interval {
+            // バッファにエントリがある場合なので同期してしまう。
             track!(self.sync())?;
         } else {
             for _ in 0..GC_COUNT_IN_SIDE_JOB {
@@ -167,6 +168,10 @@ where
         &self.metrics
     }
 
+    pub fn is_dirty(&self) -> bool {
+        self.ring_buffer.is_dirty()
+    }
+
     /// GC処理を一単位実行する.
     fn gc_once(&mut self, index: &mut LumpIndex) -> Result<()> {
         if self.gc_queue.is_empty() && self.ring_buffer.capacity() < self.ring_buffer.usage() * 2 {
@@ -248,10 +253,11 @@ where
         if self.gc_after_append {
             track!(self.gc_once(index))?; // レコード追記に合わせてGCを一単位行うことでコストを償却する
         }
-        track!(self.try_sync())?;
         Ok(())
     }
 
+    // ジャーナルキュー（のバッファ）にエントリを追加する。
+    // 十分なエントリが溜まっていれば同期を行う。
     fn append_record<B>(&mut self, index: &mut LumpIndex, record: &JournalRecord<B>) -> Result<()>
     where
         B: AsRef<[u8]>,
@@ -260,6 +266,7 @@ where
         if let Some((lump_id, portion)) = embedded {
             index.insert(lump_id, Portion::Journal(portion));
         }
+        track!(self.try_sync())?;
         Ok(())
     }
 
diff --git a/src/storage/journal/ring_buffer.rs b/src/storage/journal/ring_buffer.rs
index 1b966c2..9023764 100644
--- a/src/storage/journal/ring_buffer.rs
+++ b/src/storage/journal/ring_buffer.rs
@@ -94,6 +94,10 @@ impl<N: NonVolatileMemory> JournalRingBuffer<N> {
         &self.metrics
     }
 
+    pub fn is_dirty(&self) -> bool {
+        self.nvm.is_dirty()
+    }
+
     /// 指定位置に埋め込まれたlumpデータの読み込みを行う.
     ///
     /// データの妥当性検証は`cannyls`内では行わない.
diff --git a/src/storage/mod.rs b/src/storage/mod.rs
index eb71e3f..64076d1 100644
--- a/src/storage/mod.rs
+++ b/src/storage/mod.rs
@@ -83,11 +83,18 @@ where
     data_region: DataRegion<N>,
     lump_index: LumpIndex,
     metrics: StorageMetrics,
+    safe_release_mode: bool,
 }
 impl<N> Storage<N>
 where
     N: NonVolatileMemory,
 {
+    /// [issue28](https://github.com/frugalos/cannyls/issue28)
+    pub fn enable_safe_release_mode(&mut self, enabling: bool) {
+        self.safe_release_mode = enabling;
+        self.data_region.enable_safe_release_mode(enabling);
+    }
+
     pub(crate) fn new(
         header: StorageHeader,
         journal_region: JournalRegion<N>,
@@ -101,6 +108,7 @@ where
             data_region,
             lump_index,
             metrics,
+            safe_release_mode: false,
         }
     }
 
@@ -176,6 +184,12 @@ where
         self.lump_index.list_range(range)
     }
 
+    fn release_pending_portions(&mut self) {
+        if !self.journal_region.is_dirty() {
+            self.data_region.release_pending_portions();
+        }
+    }
+
     /// lumpを保存する.
     ///
     /// 既に同じIDのlumpが存在する場合にはデータが上書きされる.
@@ -211,6 +225,10 @@ where
             }
         }
         self.metrics.put_lumps_at_running.increment();
+
+        if self.safe_release_mode {
+            self.release_pending_portions();
+        }
         Ok(!updated)
     }
 
@@ -224,7 +242,11 @@ where
     /// 不整合ないしI/O周りで致命的な問題が発生している可能性があるので、
     /// 以後はこのインスタンスの使用を中止するのが望ましい.
     pub fn delete(&mut self, lump_id: &LumpId) -> Result<bool> {
-        track!(self.delete_if_exists(lump_id, true))
+        let result = track!(self.delete_if_exists(lump_id, true))?;
+        if self.safe_release_mode {
+            self.release_pending_portions();
+        }
+        Ok(result)
     }
 
     /// LumpIdのrange [start..end) を用いて、これに含まれるLumpIdを全て削除する。
@@ -248,9 +270,9 @@ where
         // ジャーナル領域に範囲削除レコードを一つ書き込むため、一度のディスクアクセスが起こる。
         // 削除レコードを範囲分書き込むわけ *ではない* ため、複数回のディスクアクセスは発生しない。
         track!(self
-               .journal_region
-               .records_delete_range(&mut self.lump_index, range))?;
-        
+            .journal_region
+            .records_delete_range(&mut self.lump_index, range))?;
+
         for lump_id in &targets {
             if let Some(portion) = self.lump_index.remove(lump_id) {
                 self.metrics.delete_lumps.increment();
@@ -262,7 +284,12 @@ where
                     self.data_region.delete(portion);
                 }
             }
-        }        
+        }
+
+        if self.safe_release_mode {
+            track!(self.journal_region.sync())?;
+            self.release_pending_portions();
+        }
 
         Ok(targets)
     }

From bf2ea3865cec8af017f64b87b16c31e4097dba50 Mon Sep 17 00:00:00 2001
From: yuuya uezato <yuuya_uezato@dwango.co.jp>
Date: Tue, 14 May 2019 01:39:00 +0900
Subject: [PATCH 2/4] add tests

---
 src/nvm/file.rs                               |   2 +-
 .../allocator/data_portion_allocator.rs       |   2 +-
 src/storage/builder.rs                        |   7 +-
 src/storage/data_region.rs                    | 108 +++++++++++++++---
 src/storage/journal/nvm_buffer.rs             |   6 +-
 src/storage/journal/region.rs                 |  25 +++-
 src/storage/journal/ring_buffer.rs            |   4 -
 src/storage/mod.rs                            |  88 ++++++++++++--
 8 files changed, 202 insertions(+), 40 deletions(-)

diff --git a/src/nvm/file.rs b/src/nvm/file.rs
index 0e7e1f7..12693fa 100644
--- a/src/nvm/file.rs
+++ b/src/nvm/file.rs
@@ -487,7 +487,7 @@ mod tests {
         let mut parent = dir.as_ref();
         while let Some(p) = parent.parent() {
             parent = p;
-        };
+        }
         assert!(create_parent_directories(parent).is_ok());
         Ok(())
     }
diff --git a/src/storage/allocator/data_portion_allocator.rs b/src/storage/allocator/data_portion_allocator.rs
index 756343b..948c617 100644
--- a/src/storage/allocator/data_portion_allocator.rs
+++ b/src/storage/allocator/data_portion_allocator.rs
@@ -195,7 +195,7 @@ impl DataPortionAllocator {
     //    現在の実装では `nth(0)` を用いているため、
     //    フリーリスト内の相異なる部分領域が互いに素であるという前提が必要である。
     //    ただしこの前提は通常のCannyLSの使用であれば成立する。
-    fn is_allocated_portion(&self, portion: &DataPortion) -> bool {
+    pub(crate) fn is_allocated_portion(&self, portion: &DataPortion) -> bool {
         let key = EndBasedFreePortion(FreePortion::new(portion.start, 0));
         if let Some(next) = self.end_to_free.range((Excluded(&key), Unbounded)).nth(0) {
             // 終端位置が `portion.start` を超えるfree portionのうち最小のもの `next` については
diff --git a/src/storage/builder.rs b/src/storage/builder.rs
index e8947e3..11b6e06 100644
--- a/src/storage/builder.rs
+++ b/src/storage/builder.rs
@@ -37,7 +37,9 @@ impl StorageBuilder {
         }
     }
 
-    /// [issue28](https://github.com/frugalos/cannyls/issue28)
+    /// 安全にリソースを解放する状態でStorageを作成する。
+    ///
+    /// 安全な解放については [wiki](https://github.com/frugalos/cannyls/wiki/Safe-Release-Mode) を参考のこと。
     pub fn enable_safe_release_mode(&mut self) -> &mut Self {
         self.safe_release_mode = true;
         self
@@ -207,7 +209,8 @@ impl StorageBuilder {
         ))?;
 
         // データ領域を準備
-        let data_region = DataRegion::new(&self.metrics, allocator, data_nvm);
+        let mut data_region = DataRegion::new(&self.metrics, allocator, data_nvm);
+        data_region.enable_safe_release_mode(self.safe_release_mode);
 
         let metrics = StorageMetrics::new(
             &self.metrics,
diff --git a/src/storage/data_region.rs b/src/storage/data_region.rs
index 7ae2cf3..3b842d0 100644
--- a/src/storage/data_region.rs
+++ b/src/storage/data_region.rs
@@ -20,7 +20,11 @@ pub struct DataRegion<N> {
     block_size: BlockSize,
     metrics: DataRegionMetrics,
     safe_release_mode: bool,
-    reserved_portions: Vec<DataPortion>,
+
+    // 削除処理によってlump indexから外されたが
+    // まだアロケータに通知して解放することができない
+    // portionたちのバッファ
+    pending_portions: Vec<DataPortion>,
 }
 impl<N> DataRegion<N>
 where
@@ -37,16 +41,18 @@ where
             block_size,
             metrics: DataRegionMetrics::new(metric_builder, capacity, allocator_metrics),
             safe_release_mode: false,
-            reserved_portions: Vec::new(),
+            pending_portions: Vec::new(),
         }
     }
 
     /// 安全にリソースを解放するモードに移行するためのメソッド。
     /// * `true`を渡すと、安全な解放モードに入る。
-    ///    * 安全な解放については [issue28](https://github.com/frugalos/cannyls/issue28) を参考のこと。
-    /// * `false`を渡すと、削除されたデータポーションが即座にアロケータから解放される。
+    ///    * 安全な解放については [wiki](https://github.com/frugalos/cannyls/wiki/Safe-Release-Mode) を参考のこと。
+    /// * `false`を渡すと、従来の解放モードに入る。
+    ///    * このモードでは、削除されたデータポーションが即座にアロケータから解放される。
+    ///    * [issue28](https://github.com/frugalos/cannyls/issues28)があるため安全ではない。
     pub fn enable_safe_release_mode(&mut self, enabling: bool) {
-        if !enabling && !self.reserved_portions.is_empty() {
+        if !enabling && !self.pending_portions.is_empty() {
             // 削除対象ポーションが存在する状況で即時削除モードに切り替える場合は
             // この段階で削除を行う
             self.release_pending_portions();
@@ -54,6 +60,13 @@ where
         self.safe_release_mode = enabling;
     }
 
+    /// 安全にリソースを解放するモードに入っているかどうかを返す。
+    /// * `true`なら安全な解放モード
+    /// * `false`なら従来の解放モード
+    pub fn is_in_safe_release_mode(&self) -> bool {
+        self.safe_release_mode
+    }
+
     /// データ領域のメトリクスを返す.
     pub fn metrics(&self) -> &DataRegionMetrics {
         &self.metrics
@@ -107,15 +120,22 @@ where
     /// 現在の実行スレッドがパニックする.
     pub fn delete(&mut self, portion: DataPortion) {
         if self.safe_release_mode {
-            self.reserved_portions.push(portion);
+            self.pending_portions.push(portion);
         } else {
             self.allocator.release(portion);
         }
     }
 
+    /// 解放を遅延させているデータポーションをアロケータに送ることで全て解放する。
+    ///
+    /// # 安全解放モードで呼び出す前提条件
+    /// 解放されるデータポーションに対応する削除レコードが、全て永続化されていること。
+    ///
+    /// # メモ
+    /// 通常の解放モードで呼び出しても効果はない。
     pub fn release_pending_portions(&mut self) {
         if self.safe_release_mode {
-            for p in ::std::mem::replace(&mut self.reserved_portions, Vec::new()) {
+            for p in ::std::mem::replace(&mut self.pending_portions, Vec::new()) {
                 self.allocator.release(p);
             }
         }
@@ -132,6 +152,11 @@ where
     fn block_count(&self, size: u32) -> u32 {
         (size + u32::from(self.block_size.as_u16()) - 1) / u32::from(self.block_size.as_u16())
     }
+
+    #[cfg(test)]
+    pub fn is_allocated_portion(&self, portion: &DataPortion) -> bool {
+        self.allocator.is_allocated_portion(portion)
+    }
 }
 
 #[derive(Debug, Clone)]
@@ -202,17 +227,59 @@ mod tests {
     use metrics::DataAllocatorMetrics;
     use nvm::MemoryNvm;
 
+    macro_rules! make_data_region_on_memory {
+        ($capacity:expr, $block_size:expr) => {{
+            let metrics = MetricBuilder::new();
+            let allocator = track!(DataPortionAllocator::build(
+                DataAllocatorMetrics::new(&metrics, $capacity, $block_size),
+                iter::empty(),
+            ))?;
+            let nvm = MemoryNvm::new(vec![0; $capacity as usize]);
+            DataRegion::new(&metrics, allocator, nvm)
+        }};
+    }
+
     #[test]
     fn data_region_works() -> TestResult {
         let capacity = 10 * 1024;
         let block_size = BlockSize::min();
-        let metrics = MetricBuilder::new();
-        let allocator = track!(DataPortionAllocator::build(
-            DataAllocatorMetrics::new(&metrics, capacity, block_size),
-            iter::empty(),
-        ))?;
-        let nvm = MemoryNvm::new(vec![0; capacity as usize]);
-        let mut region = DataRegion::new(&metrics, allocator, nvm);
+        let mut region = make_data_region_on_memory!(capacity, block_size);
+
+        // put
+        let mut data = DataRegionLumpData::new(3, block_size);
+        data.as_bytes_mut().copy_from_slice("foo".as_bytes());
+        let portion = track!(region.put(&data))?;
+
+        // get
+        assert_eq!(
+            region.get(portion).ok().map(|d| d.as_bytes().to_owned()),
+            Some("foo".as_bytes().to_owned())
+        );
+        Ok(())
+    }
+
+    #[test]
+    fn enabling_and_confirming_safe_release_mode_work() -> TestResult {
+        let capacity = 10 * 1024;
+        let block_size = BlockSize::min();
+        let mut region = make_data_region_on_memory!(capacity, block_size);
+
+        // デフォルトでは安全解放モードを使わない。
+        assert_eq!(region.is_in_safe_release_mode(), false);
+
+        region.enable_safe_release_mode(true);
+        assert_eq!(region.is_in_safe_release_mode(), true);
+
+        Ok(())
+    }
+
+    #[test]
+    fn delayed_releasing_works() -> TestResult {
+        let capacity = 10 * 1024;
+        let block_size = BlockSize::min();
+        let mut region = make_data_region_on_memory!(capacity, block_size);
+
+        region.enable_safe_release_mode(true);
 
         // put
         let mut data = DataRegionLumpData::new(3, block_size);
@@ -224,6 +291,19 @@ mod tests {
             region.get(portion).ok().map(|d| d.as_bytes().to_owned()),
             Some("foo".as_bytes().to_owned())
         );
+
+        region.delete(portion.clone());
+
+        // まだ解放されていない。
+        assert_eq!(region.allocator.is_allocated_portion(&portion), true);
+
+        assert_eq!(region.pending_portions.len(), 1);
+
+        region.release_pending_portions();
+
+        // 解放された。
+        assert_eq!(region.allocator.is_allocated_portion(&portion), false);
+
         Ok(())
     }
 }
diff --git a/src/storage/journal/nvm_buffer.rs b/src/storage/journal/nvm_buffer.rs
index 2533fa9..192ee27 100644
--- a/src/storage/journal/nvm_buffer.rs
+++ b/src/storage/journal/nvm_buffer.rs
@@ -82,10 +82,6 @@ impl<N: NonVolatileMemory> JournalNvmBuffer<N> {
         &self.inner
     }
 
-    pub fn is_dirty(&self) -> bool {
-        self.maybe_dirty
-    }
-
     fn is_dirty_area(&self, offset: u64, length: usize) -> bool {
         if !self.maybe_dirty || length == 0 || self.write_buf.is_empty() {
             return false;
@@ -184,7 +180,7 @@ impl<N: NonVolatileMemory> Seek for JournalNvmBuffer<N> {
 impl<N: NonVolatileMemory> Read for JournalNvmBuffer<N> {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
         if self.is_dirty_area(self.position, buf.len()) {
-            track!(self.flush_write_buf())?;
+            track!(self.sync())?;
         }
 
         let aligned_start = self.block_size().floor_align(self.position);
diff --git a/src/storage/journal/region.rs b/src/storage/journal/region.rs
index 3a641b4..e75f6ac 100644
--- a/src/storage/journal/region.rs
+++ b/src/storage/journal/region.rs
@@ -44,6 +44,11 @@ impl<N> JournalRegion<N>
 where
     N: NonVolatileMemory,
 {
+    #[cfg(test)]
+    pub fn options(&self) -> &JournalRegionOptions {
+        &self.options
+    }
+
     pub fn journal_entries(&mut self) -> Result<(u64, u64, u64, Vec<JournalEntry>)> {
         self.ring_buffer.journal_entries()
     }
@@ -152,7 +157,6 @@ where
         if self.gc_queue.is_empty() {
             track!(self.fill_gc_queue())?;
         } else if self.sync_countdown != self.options.sync_interval {
-            // バッファにエントリがある場合なので同期してしまう。
             track!(self.sync())?;
         } else {
             for _ in 0..GC_COUNT_IN_SIDE_JOB {
@@ -168,8 +172,18 @@ where
         &self.metrics
     }
 
-    pub fn is_dirty(&self) -> bool {
-        self.ring_buffer.is_dirty()
+    /// ジャーナルバッファが同期され永続化された直後の状態かどうかを返す。
+    ///
+    /// この状態は、厳密には次を意味する。
+    /// 1. 現時点までに以下四種類の操作でバッファに書き込まれたジャーナルエントリは
+    /// 全てDiskに同期されている。
+    ///   (a) records_put
+    ///   (b) records_embed
+    ///   (c) records_delete
+    ///   (d) records_delete_range
+    /// 2. ジャーナルバッファは空である。
+    pub fn is_just_synced(&self) -> bool {
+        self.sync_countdown == self.options.sync_interval
     }
 
     /// GC処理を一単位実行する.
@@ -271,10 +285,11 @@ where
     }
 
     fn try_sync(&mut self) -> Result<()> {
+        assert!(self.sync_countdown >= 1);
+        self.sync_countdown -= 1;
+
         if self.sync_countdown == 0 {
             track!(self.sync())?;
-        } else {
-            self.sync_countdown -= 1;
         }
         Ok(())
     }
diff --git a/src/storage/journal/ring_buffer.rs b/src/storage/journal/ring_buffer.rs
index 9023764..1b966c2 100644
--- a/src/storage/journal/ring_buffer.rs
+++ b/src/storage/journal/ring_buffer.rs
@@ -94,10 +94,6 @@ impl<N: NonVolatileMemory> JournalRingBuffer<N> {
         &self.metrics
     }
 
-    pub fn is_dirty(&self) -> bool {
-        self.nvm.is_dirty()
-    }
-
     /// 指定位置に埋め込まれたlumpデータの読み込みを行う.
     ///
     /// データの妥当性検証は`cannyls`内では行わない.
diff --git a/src/storage/mod.rs b/src/storage/mod.rs
index 64076d1..94cdc98 100644
--- a/src/storage/mod.rs
+++ b/src/storage/mod.rs
@@ -83,18 +83,25 @@ where
     data_region: DataRegion<N>,
     lump_index: LumpIndex,
     metrics: StorageMetrics,
-    safe_release_mode: bool,
 }
 impl<N> Storage<N>
 where
     N: NonVolatileMemory,
 {
-    /// [issue28](https://github.com/frugalos/cannyls/issue28)
+    /// 安全にリソースを解放する状態でStorageを作成する。
+    ///
+    /// 安全な解放については [wiki](https://github.com/frugalos/cannyls/wiki/Safe-Release-Mode) を参考のこと。
     pub fn enable_safe_release_mode(&mut self, enabling: bool) {
-        self.safe_release_mode = enabling;
         self.data_region.enable_safe_release_mode(enabling);
     }
 
+    /// 安全にリソースを解放するモードに入っているかどうかを返す。
+    /// * `true`なら安全な解放モード
+    /// * `false`なら従来の解放モード
+    pub fn is_in_safe_release_mode(&self) -> bool {
+        self.data_region.is_in_safe_release_mode()
+    }
+
     pub(crate) fn new(
         header: StorageHeader,
         journal_region: JournalRegion<N>,
@@ -108,7 +115,6 @@ where
             data_region,
             lump_index,
             metrics,
-            safe_release_mode: false,
         }
     }
 
@@ -184,8 +190,14 @@ where
         self.lump_index.list_range(range)
     }
 
+    /// ジャーナル領域が直前の操作に伴って同期済みであるならば
+    /// 遅延させている削除済みポーションを全て解放する。
+    ///
+    /// syncは実際にはバッファのflushとディスク同期の両方を行うため
+    /// is_just_synced() == trueならば
+    /// この時点までの全てのジャーナルレコードが同期済みである。
     fn release_pending_portions(&mut self) {
-        if !self.journal_region.is_dirty() {
+        if self.journal_region.is_just_synced() {
             self.data_region.release_pending_portions();
         }
     }
@@ -226,7 +238,7 @@ where
         }
         self.metrics.put_lumps_at_running.increment();
 
-        if self.safe_release_mode {
+        if self.is_in_safe_release_mode() {
             self.release_pending_portions();
         }
         Ok(!updated)
@@ -243,7 +255,7 @@ where
     /// 以後はこのインスタンスの使用を中止するのが望ましい.
     pub fn delete(&mut self, lump_id: &LumpId) -> Result<bool> {
         let result = track!(self.delete_if_exists(lump_id, true))?;
-        if self.safe_release_mode {
+        if self.is_in_safe_release_mode() {
             self.release_pending_portions();
         }
         Ok(result)
@@ -286,7 +298,7 @@ where
             }
         }
 
-        if self.safe_release_mode {
+        if self.is_in_safe_release_mode() {
             track!(self.journal_region.sync())?;
             self.release_pending_portions();
         }
@@ -873,4 +885,64 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn safe_releasing_works() -> TestResult {
+        let dir = track_io!(TempDir::new("cannyls_test"))?;
+
+        let nvm = track!(FileNvm::create(
+            dir.path().join("test.lusf"),
+            BlockSize::min().ceil_align(512 * 0x1000 * 10)
+        ))?;
+        let mut storage = track!(StorageBuilder::new()
+            .journal_region_ratio(0.5)
+            .enable_safe_release_mode()
+            .create(nvm))?;
+        assert!(storage.is_in_safe_release_mode());
+
+        assert_eq!(storage.journal_region.options().sync_interval, 0x1000);
+
+        let mut portions = Vec::new();
+
+        for i in 0..(0x1000 / 2 - 1) {
+            track!(storage.put(&LumpId::new(i), &zeroed_data(42)))?;
+            let portion = storage
+                .lump_index
+                .get(&LumpId::new(i))
+                .expect("should succeed");
+            portions.push(portion.clone());
+            track!(storage.delete(&LumpId::new(i)))?;
+        }
+        track!(storage.put(&LumpId::new(10000), &zeroed_data(42)))?;
+        let portion = storage
+            .lump_index
+            .get(&LumpId::new(10000))
+            .expect("should succeed");
+        portions.push(portion.clone());
+
+        for p in &portions {
+            if let Portion::Data(portion) = p {
+                assert!(storage.data_region.is_allocated_portion(portion));
+            } else {
+                unreachable!("since we've put unembedded lump data");
+            }
+        }
+
+        assert_eq!(storage.journal_region.is_just_synced(), false);
+
+        // このDeleteによって0x1000個目のエントリが書き込まれるため
+        // ジャーナル領域の同期が行われる。
+        track!(storage.delete(&LumpId::new(10000)))?;
+        assert_eq!(storage.journal_region.is_just_synced(), true);
+
+        for p in &portions {
+            if let Portion::Data(portion) = p {
+                assert!(!storage.data_region.is_allocated_portion(portion));
+            } else {
+                unreachable!("since we've put unembedded lump data");
+            }
+        }
+
+        Ok(())
+    }
 }

From 52ca0593e37e3d34ae17f1c5fabe0404df3f11cb Mon Sep 17 00:00:00 2001
From: yuuya uezato <yuuya_uezato@dwango.co.jp>
Date: Tue, 14 May 2019 17:24:46 +0900
Subject: [PATCH 3/4] =?UTF-8?q?DeleteRange=E3=82=92=E7=89=B9=E5=88=A5?=
 =?UTF-8?q?=E6=89=B1=E3=81=84?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/storage/data_region.rs | 11 +++++++++++
 src/storage/mod.rs         |  4 ++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/storage/data_region.rs b/src/storage/data_region.rs
index 3b842d0..e8f5f72 100644
--- a/src/storage/data_region.rs
+++ b/src/storage/data_region.rs
@@ -126,6 +126,17 @@ where
         }
     }
 
+    /// 指定された領域に格納されているデータを削除する.
+    ///
+    /// deleteと似ているが、こちらは安全解放モードでもpendingせず解放する。
+    /// # パニック
+    ///
+    /// `portion`で未割当の領域が指定された場合には、
+    /// 現在の実行スレッドがパニックする.
+    pub fn release_portion(&mut self, portion: DataPortion) {
+        self.allocator.release(portion);
+    }
+
     /// 解放を遅延させているデータポーションをアロケータに送ることで全て解放する。
     ///
     /// # 安全解放モードで呼び出す前提条件
diff --git a/src/storage/mod.rs b/src/storage/mod.rs
index 94cdc98..0501232 100644
--- a/src/storage/mod.rs
+++ b/src/storage/mod.rs
@@ -293,13 +293,13 @@ where
                     // DataRegion::deleteはメモリアロケータに対する解放要求をするのみで
                     // ディスクにアクセスすることはない。
                     // （管理領域から外すだけで、例えばディスク上の値を0クリアするようなことはない）
-                    self.data_region.delete(portion);
+                    self.data_region.release_portion(portion);
                 }
             }
         }
 
         if self.is_in_safe_release_mode() {
-            track!(self.journal_region.sync())?;
+            track!(self.journal_sync())?;
             self.release_pending_portions();
         }
 

From 57c27510a5af44e4d73c9bcf38bd147ba5c3a3ef Mon Sep 17 00:00:00 2001
From: yuuya uezato <yuuya_uezato@dwango.co.jp>
Date: Thu, 18 Jul 2019 18:49:58 +0900
Subject: [PATCH 4/4] fix mistakes

---
 src/nvm/file.rs                   | 2 +-
 src/storage/journal/nvm_buffer.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/nvm/file.rs b/src/nvm/file.rs
index 12693fa..0e7e1f7 100644
--- a/src/nvm/file.rs
+++ b/src/nvm/file.rs
@@ -487,7 +487,7 @@ mod tests {
         let mut parent = dir.as_ref();
         while let Some(p) = parent.parent() {
             parent = p;
-        }
+        };
         assert!(create_parent_directories(parent).is_ok());
         Ok(())
     }
diff --git a/src/storage/journal/nvm_buffer.rs b/src/storage/journal/nvm_buffer.rs
index 192ee27..0b699a2 100644
--- a/src/storage/journal/nvm_buffer.rs
+++ b/src/storage/journal/nvm_buffer.rs
@@ -180,7 +180,7 @@ impl<N: NonVolatileMemory> Seek for JournalNvmBuffer<N> {
 impl<N: NonVolatileMemory> Read for JournalNvmBuffer<N> {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
         if self.is_dirty_area(self.position, buf.len()) {
-            track!(self.sync())?;
+            track!(self.flush_write_buf())?;
         }
 
         let aligned_start = self.block_size().floor_align(self.position);