From fb0a76a9e2010def0348e63bfc1a6b2c4866bfa3 Mon Sep 17 00:00:00 2001
From: Hui Xiao <huixiao@fb.com>
Date: Thu, 6 Jan 2022 10:09:13 -0800
Subject: [PATCH] Always check previous conditionally unchecked status due to
 shortcut evaluation in BlockBasedTableBuilder::WriteIndexBlock (#9349)

Summary:
Note: part of https://github.com/facebook/rocksdb/pull/9342
**Context/Summary:**
Due to shortcut evaluation in `ok() && s.IsIncomplete()`, status `s` remains unchecked if `ok()==false`, which is the case in https://app.circleci.com/pipelines/github/facebook/rocksdb/10718/workflows/429f7ad4-6b9a-446b-b9b3-710d51b90409/jobs/265508 revealed by the change in the corresponding PR https://github.com/facebook/rocksdb/pull/9342.

As suggested by reviewers, separation and clarification of status checking for partitioned index building from general table building status is added.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9349

Test Plan:
- The newly added if-else code is an equivalent translation of the existing logic plus always checking the conditionally unchecked status so relying on existing tests should be fine
- https://github.com/facebook/rocksdb/pull/9342's `[build-linux-shared_lib-alt_namespace-status_checked](https://app.circleci.com/pipelines/github/facebook/rocksdb/10721/workflows/a200efe0-d545-4075-8c42-26dd3dc00f27/jobs/265625)` test should now pass after rebasing on this change

Reviewed By: pdillinger

Differential Revision: D33377223

Pulled By: hx235

fbshipit-source-id: cb81da9709ae9185e9cea89776e3012e915d6ef9
---
 table/block_based/block_based_table_builder.cc | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc
index cf1ea5db6..fb25a7ff9 100644
--- a/table/block_based/block_based_table_builder.cc
+++ b/table/block_based/block_based_table_builder.cc
@@ -1613,13 +1613,21 @@ void BlockBasedTableBuilder::WriteIndexBlock(
   }
   // If there are more index partitions, finish them and write them out
   if (index_builder_status.IsIncomplete()) {
-    Status s = Status::Incomplete();
-    while (ok() && s.IsIncomplete()) {
-      s = rep_->index_builder->Finish(&index_blocks, *index_block_handle);
-      if (!s.ok() && !s.IsIncomplete()) {
+    bool index_building_finished = false;
+    while (ok() && !index_building_finished) {
+      Status s =
+          rep_->index_builder->Finish(&index_blocks, *index_block_handle);
+      if (s.ok()) {
+        index_building_finished = true;
+      } else if (s.IsIncomplete()) {
+        // More partitioned index after this one
+        assert(!index_building_finished);
+      } else {
+        // Error
         rep_->SetStatus(s);
         return;
       }
+
       if (rep_->table_options.enable_index_compression) {
         WriteBlock(index_blocks.index_block_contents, index_block_handle,
                    BlockType::kIndex);