From: Takanori Nakao <nakao.takanori@jp.fujitsu.com>
Date: Mon, 15 Jun 2015 07:41:19 +0000 (+0900)
Subject: erasure code: fix shec's recovery issues found in teuthology test
X-Git-Tag: v9.0.3~74^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9bdf3e6c289dab34dfd78d717eea051f40cc78f8;p=ceph.git

erasure code: fix shec's recovery issues found in teuthology test

Fixed shec's recovery issues caused by unexpected arguments in
minimum_to_decode() / decode() functions.
We misunderstood as follows.

 (1) want_to_read and available_chunks do not share osds.
 (2) osds that are not included in want_to_read are always included
     in available_chunks.

Signed-off-by: Takanori Nakao <nakao.takanori@jp.fujitsu.com>
---

diff --git a/src/erasure-code/shec/ErasureCodeShec.cc b/src/erasure-code/shec/ErasureCodeShec.cc
index 61a51d5a3c34..ffd8e59c594f 100644
--- a/src/erasure-code/shec/ErasureCodeShec.cc
+++ b/src/erasure-code/shec/ErasureCodeShec.cc
@@ -31,8 +31,6 @@ extern "C" {
 #include "jerasure/include/jerasure.h"
 #include "jerasure/include/galois.h"
 
-#define talloc(type, num) (type *) malloc(sizeof(type)*(num))
-
 extern int calc_determinant(int *matrix, int dim);
 extern int* reed_sol_vandermonde_coding_matrix(int k, int m, int w);
 }
@@ -41,8 +39,6 @@ extern int* reed_sol_vandermonde_coding_matrix(int k, int m, int w);
 #undef dout_prefix
 #define dout_prefix _prefix(_dout)
 
-#define talloc(type, num) (type *) malloc(sizeof(type)*(num))
-
 static ostream& _prefix(std::ostream* _dout)
 {
   return *_dout << "ErasureCodeShec: ";
@@ -89,7 +85,7 @@ unsigned int ErasureCodeShec::get_chunk_size(unsigned int object_size) const
   return padded_length / k;
 }
 
-int ErasureCodeShec::minimum_to_decode(const set<int> &want_to_decode,
+int ErasureCodeShec::minimum_to_decode(const set<int> &want_to_read,
 				       const set<int> &available_chunks,
 				       set<int> *minimum_chunks)
 {
@@ -99,43 +95,51 @@ int ErasureCodeShec::minimum_to_decode(const set<int> &want_to_decode,
     if (*it < 0 || k+m <= *it) return -EINVAL;
   }
 
-  if (includes(available_chunks.begin(), available_chunks.end(),
-	       want_to_decode.begin(), want_to_decode.end())) {
-    *minimum_chunks = want_to_decode;
-  } else {
-    int erased[k + m];
-    int avails[k + m];
-    int minimum[k + m];
-    int dm_ids[k];
+  for (set<int>::iterator it = want_to_read.begin(); it != want_to_read.end(); ++it){
+    if (*it < 0 || k+m <= *it) return -EINVAL;
+  }
 
-    memset(erased, 0, sizeof(erased));
-    memset(dm_ids, 0, sizeof(dm_ids));
+  int want[k + m];
+  int avails[k + m];
+  int minimum[k + m];
 
-    for (int i = 0; i < k + m; i++) {
-      erased[i] = 0;
-      if (available_chunks.find(i) == available_chunks.end()) {
-	if (want_to_decode.count(i) > 0) {
-	  erased[i] = 1;
-	}
-	avails[i] = 0;
-      } else {
-	avails[i] = 1;
-      }
-    }
+  memset(want, 0, sizeof(want));
+  memset(avails, 0, sizeof(avails));
+  memset(minimum, 0, sizeof(minimum));
+  (*minimum_chunks).clear();
 
-    if (shec_make_decoding_matrix(true, erased, avails, 0, dm_ids, minimum) < 0) {
+  for (set<int>::const_iterator i = want_to_read.begin();
+       i != want_to_read.end();
+       ++i) {
+    want[*i] = 1;
+  }
+
+  for (set<int>::const_iterator i = available_chunks.begin();
+       i != available_chunks.end();
+       ++i) {
+    avails[*i] = 1;
+  }
+
+  {
+    int decoding_matrix[k*k];
+    int dm_row[k];
+    int dm_column[k];
+    memset(decoding_matrix, 0, sizeof(decoding_matrix));
+    memset(dm_row, 0, sizeof(dm_row));
+    memset(dm_column, 0, sizeof(dm_column));
+    if (shec_make_decoding_matrix(true, want, avails, decoding_matrix, dm_row, dm_column, minimum) < 0) {
       return -EIO;
     }
+  }
 
-    for (int i = 0; i < k + m; i++) {
-      if (minimum[i] == 1) minimum_chunks->insert(i);
-    }
+  for (int i = 0; i < k + m; i++) {
+    if (minimum[i] == 1) minimum_chunks->insert(i);
   }
 
   return 0;
 }
 
-int ErasureCodeShec::minimum_to_decode_with_cost(const set<int> &want_to_decode,
+int ErasureCodeShec::minimum_to_decode_with_cost(const set<int> &want_to_read,
 						 const map<int, int> &available,
 						 set<int> *minimum_chunks)
 {
@@ -146,7 +150,7 @@ int ErasureCodeShec::minimum_to_decode_with_cost(const set<int> &want_to_decode,
        ++i)
     available_chunks.insert(i->first);
 
-  return minimum_to_decode(want_to_decode, available_chunks, minimum_chunks);
+  return minimum_to_decode(want_to_read, available_chunks, minimum_chunks);
 }
 
 int ErasureCodeShec::encode(const set<int> &want_to_encode,
@@ -538,184 +542,270 @@ int* ErasureCodeShec::shec_reedsolomon_coding_matrix(int is_single)
   return matrix;
 }
 
-int ErasureCodeShec::shec_make_decoding_matrix(bool prepare, int *erased, int *avails,
-                                               int *decoding_matrix, int *dm_ids, int *minimum)
+int ErasureCodeShec::shec_make_decoding_matrix(bool prepare, int *want_, int *avails,
+                                               int *decoding_matrix, int *dm_row, int *dm_column,
+                                               int *minimum)
 {
-  int i, j, det = 0;
-  int ek;
-  int *tmpmat = NULL, tmprow[k+m], element, dup, mindup;
+  int mindup = k+1, minp = k+1;
+  int want[k + m];
+  for (int i = 0; i < k + m; ++i) {
+    want[i] = want_[i];
+  }
 
-  for (i = 0, j = 0, ek = 0; i < k; i++) {
-    if (erased[i] == 1) {
-      ek++;
-    } else {
-      dm_ids[j] = i;
-      j++;
+  for (int i = 0; i < m; ++i) {
+    if (want[i + k] && !avails[i + k]) {
+      for (int j=0; j < k; ++j) {
+        if (matrix[i * k + j] > 0) {
+          want[j] = 1;
+        }
+      }
     }
   }
 
-  tmpmat = talloc(int, k*k);
-  if (tmpmat == NULL) { return -1; }
-  for (i = 0; i < k-ek; i++) {
-    for (j = 0; j < k; j++) tmpmat[i*k+j] = 0;
-    tmpmat[i*k+dm_ids[i]] = 1;
-  }
+  for (unsigned long long pp = 0; pp < (1ull << m); ++pp) {
 
-  if (ek > m){
-    free(tmpmat);
-    return -1;
-  }
+    // select parity chunks
+    int ek = 0;
+    int p[m];
+    for (int i=0; i < m; ++i) {
+      if (pp & (1ull << i)) {
+        p[ek++] = i;
+      }
+    }
+    if (ek > minp) {
+      continue;
+    }
 
-  mindup = k+1;
-  int minc[ek];
-  for (i=0; i<ek; i++){
-    minc[i] = -1;
-  }
-  int p[ek];
-  int pp[k+m];
-  for (i=0; i<ek; i++){
-    pp[i] = 1;
-  }
-  for (i=ek; i<m; i++){
-    pp[i] = 0;
-  }
+    // Are selected parity chunks avail?
+    bool ok = true;
+    for (int i = 0; i < ek && ok; i++) {
+      if (!avails[k+p[i]]) {
+        ok = false;
+        break;
+      }
+    }
+
+    if (!ok) {
+      continue;
+    }
 
-  do {
-    i=0;
-    for (j=0; j<m; j++){
-      if (pp[j]){
-        p[i++] = j;
+    int tmprow[k + m];
+    int tmpcolumn[k];
+    for (int i = 0; i < k + m; i++) {
+      tmprow[i] = 0;
+    }
+    for (int i = 0; i < k; i++) {
+      tmpcolumn[i] = 0;
+    }
+
+    for (int i=0; i < k; i++) {
+      if (want[i] && !avails[i]) {
+        tmpcolumn[i] = 1;
       }
     }
 
-    bool ok = true;
-    for (i = 0; i < ek; i++) {
-      if (erased[k+p[i]] == 1 || avails[k+p[i]] == 0) ok = false;
-      for (j = 0; j < k; j++) {
-        element = matrix[(p[i])*k+j];
+    // Parity chunks which are used to recovery erased data chunks, are added to tmprow.
+    for (int i = 0; i < ek; i++) {
+      tmprow[k + p[i]] = 1;
+      for (int j = 0; j < k; j++) {
+        int element = matrix[(p[i]) * k + j];
         if (element != 0) {
-          if (erased[j] == 0 && avails[j] == 0) ok = false;
+          tmpcolumn[j] = 1;
+        }
+        if (element != 0 && avails[j] == 1) {
+          tmprow[j] = 1;
         }
       }
     }
-    if (ok == false) continue;
 
-    for (i = 0; i < k+m; i++) tmprow[i] = 0;
-    for (i = 0; i < m; i++) {
-      if (erased[k+i] == 1) {
-        for (j = 0; j < k; j++) {
-          if (matrix[i*k+j] != 0 && erased[j] == 0) tmprow[j] = 1;
-        }
+    int dup_row = 0, dup_column = 0, dup = 0;
+    for (int i = 0; i < k + m; i++) {
+      if (tmprow[i]) {
+        dup_row++;
       }
     }
-    for (i = 0; i < ek; i++) {
-      tmprow[k+p[i]] = 1;
-      for (j = 0; j < k; j++) {
-        element = matrix[(p[i])*k+j];
-        tmpmat[(k-ek+i)*k+j] = element;
-        if (element != 0 && erased[j] == 0) tmprow[j] = 1;
+
+    for (int i = 0; i < k; i++) {
+      if (tmpcolumn[i]) {
+        dup_column++;
       }
     }
-    dup = 0;
-    for (j = 0; j < k; j++) {
-      if (tmprow[j] > 0) dup++;
+
+    if (dup_row != dup_column) {
+      continue;
+    }
+    dup = dup_row;
+    if (dup == 0) {
+      mindup = dup;
+      for (int i = 0; i < k; i++) {
+        dm_row[i] = -1;
+      }
+      for (int i = 0; i < k; i++) {
+        dm_column[i] = -1;
+      }
+      break;
     }
+
+    // minimum is updated.
     if (dup < mindup) {
-      det = calc_determinant(tmpmat, k);
+      int tmpmat[dup * dup];
+      {
+        for (int i = 0, row = 0; i < k + m; i++) {
+          if (tmprow[i]) {
+            for (int j = 0, column = 0; j < k; j++) {
+              if (tmpcolumn[j]) {
+                if (i < k) {
+                  tmpmat[row * dup + column] = (i == j ? 1 : 0);
+                } else {
+                  tmpmat[row * dup + column] = matrix[(i - k) * k + j];
+                }
+                column++;
+              }
+            }
+            row++;
+          }
+        }
+      }
+      int det = calc_determinant(tmpmat, dup);
+
       if (det != 0) {
+        int row_id = 0;
+        int column_id = 0;
+        for (int i = 0; i < k; i++) {
+          dm_row[i] = -1;
+        }
+        for (int i = 0; i < k; i++) {
+          dm_column[i] = -1;
+        }
+
         mindup = dup;
-        for (int i=0; i<ek; i++){
-          minc[i] = p[i];
+        for (int i=0; i < k + m; i++) {
+          if (tmprow[i]) {
+            dm_row[row_id++] = i;
+          }
         }
+        for (int i=0; i < k; i++) {
+          if (tmpcolumn[i]) {
+            dm_column[column_id++] = i;
+          }
+        }
+        minp = ek;
       }
     }
-  } while (std::prev_permutation(pp, pp+m));
+  }
 
-  if (minc[0] == -1 && mindup == k+1) {
+
+  if (mindup == k+1) {
     fprintf(stderr, "shec_make_decoding_matrix(): can't find recover matrix.\n");
-    free(tmpmat);
     return -1;
   }
 
-  for (i = 0; i < k+m; i++) minimum[i] = 0;
-  for (i = 0; i < m; i++) {
-    if (erased[k+i] == 1) {
-      for (j = 0; j < k; j++) {
-        if (matrix[i*k+j] != 0 && erased[j] == 0) minimum[j] = 1;
-      }
+  for (int i = 0; i < k + m; i++) {
+    minimum[i] = 0;
+  }
+
+  for (int i=0; i < k && dm_row[i] != -1; i++) {
+    minimum[dm_row[i]] = 1;
+  }
+
+  for (int i = 0; i < k; ++i) {
+    if (want[i] && avails[i]) {
+      minimum[i] = 1;
     }
   }
-  for (i = 0; i < ek; i++) {
-    dm_ids[k-ek+i] = k+minc[i];
-    minimum[k+minc[i]] = 1;
-    for (j = 0; j < k; j++) {
-      element = matrix[(minc[i])*k+j];
-      tmpmat[(k-ek+i)*k+j] = element;
-      if (element != 0 && erased[j] == 0) minimum[j] = 1;
+
+  for (int i = 0; i < m; ++i) {
+    if (want[k + i] && avails[k + i] && !minimum[k + i]) {
+      for (int j = 0; j < k; ++j) {
+        if (matrix[i * k + j] > 0 && !want[j]) {
+          minimum[k + i] = 1;
+          break;
+        }
+      }
     }
   }
 
-  if (prepare == true) {
-    free(tmpmat);
+  if (mindup == 0) {
     return 0;
   }
 
-  i = jerasure_invert_matrix(tmpmat, decoding_matrix, k, w);
+  int tmpmat[mindup * mindup];
+  for (int i=0; i < mindup; i++) {
+    for (int j=0; j < mindup; j++) {
+      if (dm_row[i] < k) {
+        tmpmat[i * mindup + j] = (dm_row[i] == dm_column[j] ? 1 : 0);
+      } else {
+        tmpmat[i * mindup + j] = matrix[(dm_row[i] - k) * k + dm_column[j]];
+      }
+    }
+    if (dm_row[i] < k) {
+      for (int j = 0; j < mindup; j++) {
+        if (dm_row[i] == dm_column[j]) {
+          dm_row[i] = j;
+        }
+      }
+    } else {
+      dm_row[i] -= (k - mindup);
+    }
+  }
+
+  if (prepare) {
+    return 0;
+  }
 
-  free(tmpmat);
+  int ret = jerasure_invert_matrix(tmpmat, decoding_matrix, mindup, w);
 
-  return i;
+  return ret;
 }
 
-int ErasureCodeShec::shec_matrix_decode(int *erased, int *avails, char **data_ptrs,
+int ErasureCodeShec::shec_matrix_decode(int *want, int *avails, char **data_ptrs,
                                         char **coding_ptrs, int size)
 {
-  int i, edd;
-  int *decoding_matrix = NULL, dm_ids[k];
+  int decoding_matrix[k*k];
+  int dm_row[k], dm_column[k];
   int minimum[k + m];
 
-  memset(dm_ids, 0, sizeof(dm_ids));
+  memset(decoding_matrix, 0, sizeof(decoding_matrix));
+  memset(dm_row, -1, sizeof(dm_row));
+  memset(dm_column, -1, sizeof(dm_column));
   memset(minimum, -1, sizeof(minimum));
 
   if (w != 8 && w != 16 && w != 32) return -1;
 
-  /* Find the number of data drives failed */
+  if (shec_make_decoding_matrix(false, want, avails, decoding_matrix,
+                                dm_row, dm_column, minimum) < 0) {
+    return -1;
+  }
 
-  edd = 0;
-  for (i = 0; i < k; i++) {
-    if (erased[i]) {
-      edd++;
+  // Get decoding matrix size
+  int dm_size = 0;
+  for (int i = 0; i < k; i++) {
+    if (dm_row[i] == -1) {
+      break;
     }
+    dm_size++;
   }
 
-  decoding_matrix = talloc(int, k*k);
-  if (decoding_matrix == NULL) { return -1; }
-
-  if (shec_make_decoding_matrix(false, erased, avails, decoding_matrix, dm_ids, minimum) < 0) {
-    free(decoding_matrix);
-    return -1;
+  char *dm_data_ptrs[dm_size];
+  for (int i = 0; i < dm_size; i++) {
+    dm_data_ptrs[i] = data_ptrs[dm_column[i]];
   }
 
-  /* Decode the data drives */
-
-  for (i = 0; edd > 0 && i < k; i++) {
-    if (erased[i]) {
-      jerasure_matrix_dotprod(k, w, decoding_matrix+(i*k),
-                              dm_ids, i, data_ptrs, coding_ptrs, size);
-      edd--;
+  // Decode the data drives
+  for (int i = 0; i < dm_size; i++) {
+    if (!avails[dm_column[i]]) {
+      jerasure_matrix_dotprod(dm_size, w, decoding_matrix + (i * dm_size),
+                              dm_row, i, dm_data_ptrs, coding_ptrs, size);
     }
   }
 
-  /* Re-encode any erased coding devices */
-
-  for (i = 0; i < m; i++) {
-    if (erased[k+i]) {
-      jerasure_matrix_dotprod(k, w, matrix+(i*k), NULL, i+k,
+  // Re-encode any erased coding devices
+  for (int i = 0; i < m; i++) {
+    if (want[k+i] && !avails[k+i]) {
+      jerasure_matrix_dotprod(k, w, matrix + (i * k), NULL, i+k,
                               data_ptrs, coding_ptrs, size);
     }
   }
 
-  if (decoding_matrix != NULL) free(decoding_matrix);
-
   return 0;
 }
diff --git a/src/erasure-code/shec/ErasureCodeShec.h b/src/erasure-code/shec/ErasureCodeShec.h
index 9c6fd33f9eed..bf07b8041937 100644
--- a/src/erasure-code/shec/ErasureCodeShec.h
+++ b/src/erasure-code/shec/ErasureCodeShec.h
@@ -84,11 +84,11 @@ public:
 
   virtual unsigned int get_chunk_size(unsigned int object_size) const;
 
-  virtual int minimum_to_decode(const set<int> &want_to_decode,
+  virtual int minimum_to_decode(const set<int> &want_to_read,
 				const set<int> &available_chunks,
 				set<int> *minimum);
 
-  virtual int minimum_to_decode_with_cost(const set<int> &want_to_decode,
+  virtual int minimum_to_decode_with_cost(const set<int> &want_to_read,
 					  const map<int, int> &available,
 					  set<int> *minimum);
 
@@ -125,8 +125,11 @@ private:
   virtual int parse(const ErasureCodeProfile &profile) = 0;
 
   virtual double shec_calc_recovery_efficiency1(int k, int m1, int m2, int c1, int c2);
-  virtual int shec_make_decoding_matrix(bool prepare, int *erased, int *avails,
-                                        int *decoding_matrix, int *dm_ids, int *minimum);
+  virtual int shec_make_decoding_matrix(bool prepare,
+                                        int *want, int *avails,
+                                        int *decoding_matrix,
+                                        int *dm_row, int *dm_column,
+                                        int *minimum);
 };
 
 class ErasureCodeShecReedSolomonVandermonde : public ErasureCodeShec {