You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@omid.apache.org by yo...@apache.org on 2019/03/03 06:31:54 UTC
[incubator-omid] 02/02: [OMID-135] - Fix compaction bug that
deletes shadow cells above LWM
This is an automated email from the ASF dual-hosted git repository.
yonigo pushed a commit to branch 1.0.1
in repository https://gitbox.apache.org/repos/asf/incubator-omid.git
commit 01ce12abec4dbcc0a3c1fe4d119ae26dba5c553e
Author: Yonatan Gottesman <yo...@gmail.com>
AuthorDate: Wed Feb 27 12:36:23 2019 +0200
[OMID-135] - Fix compaction bug that deletes shadow cells above LWM
---
.../org/apache/omid/transaction/TestCellUtils.java | 53 +++++++++++++++++
.../org/apache/omid/transaction/CellUtils.java | 68 ++++++++++++++++------
.../apache/omid/transaction/CompactorScanner.java | 3 +-
.../apache/omid/transaction/TestCompaction.java | 61 +++++++++++++++++++
4 files changed, 165 insertions(+), 20 deletions(-)
diff --git a/hbase-client/src/test/java/org/apache/omid/transaction/TestCellUtils.java b/hbase-client/src/test/java/org/apache/omid/transaction/TestCellUtils.java
index 8a689ca..cbf53d0 100644
--- a/hbase-client/src/test/java/org/apache/omid/transaction/TestCellUtils.java
+++ b/hbase-client/src/test/java/org/apache/omid/transaction/TestCellUtils.java
@@ -23,6 +23,7 @@ import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.KeyValue.Type;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.omid.HBaseShims;
+import org.apache.omid.tso.client.CellId;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
@@ -227,4 +228,56 @@ public class TestCellUtils {
assertEquals(originalQualifierLength, qualifier.length);
}
+
+ @Test(timeOut = 10_000)
+ public void testmapCellsToShadowCellsCellOrder() {
+ // Create the required data
+ final byte[] validShadowCellQualifier =
+ com.google.common.primitives.Bytes.concat(SHADOW_CELL_PREFIX, qualifier, SHADOW_CELL_SUFFIX);
+
+ final byte[] qualifier2 = Bytes.toBytes("test-qual2");
+ final byte[] validShadowCellQualifier2 =
+ com.google.common.primitives.Bytes.concat(SHADOW_CELL_PREFIX, qualifier2, SHADOW_CELL_SUFFIX);
+
+ final byte[] qualifier3 = Bytes.toBytes("test-qual3");
+ final byte[] validShadowCellQualifier3 =
+ com.google.common.primitives.Bytes.concat(SHADOW_CELL_PREFIX, qualifier3, SHADOW_CELL_SUFFIX);
+
+ final byte[] qualifier4 = Bytes.toBytes("test-qual4");
+ final byte[] qualifier5 = Bytes.toBytes("test-qual5");
+ final byte[] validShadowCellQualifier5 =
+ com.google.common.primitives.Bytes.concat(SHADOW_CELL_PREFIX, qualifier5, SHADOW_CELL_SUFFIX);
+
+
+ Cell cell1 = new KeyValue(row, family, qualifier, 1, Bytes.toBytes("value")); // Default type is Put
+ Cell shadowCell1 = new KeyValue(row, family, validShadowCellQualifier, 1, Bytes.toBytes("sc-value"));
+
+ Cell cell2 = new KeyValue(row, family, qualifier2, 1, Bytes.toBytes("value2"));
+ Cell shadowCell2 = new KeyValue(row, family, validShadowCellQualifier2, 1, Bytes.toBytes("sc-value2"));
+
+ Cell cell3 = new KeyValue(row, family, qualifier3, 1, Bytes.toBytes("value3"));
+ Cell shadowCell3 = new KeyValue(row, family, validShadowCellQualifier3, 1, Bytes.toBytes("sc-value2"));
+
+ Cell cell4 = new KeyValue(row, family, qualifier4, 1, Bytes.toBytes("value4"));
+
+ Cell shadowCell5 = new KeyValue(row, family, validShadowCellQualifier5, 1, Bytes.toBytes("sc-value2"));
+
+ List<Cell> scanList = new ArrayList<>();
+ scanList.add(shadowCell5);
+ scanList.add(cell3);
+ scanList.add(cell1);
+ scanList.add(shadowCell1);
+ scanList.add(shadowCell2);
+ scanList.add(cell4);
+ scanList.add(cell2);
+ scanList.add(shadowCell3);
+ scanList.add(shadowCell5);
+
+ SortedMap<Cell, Optional<Cell>> cellsToShadowCells = CellUtils.mapCellsToShadowCells(scanList);
+ assertEquals(cellsToShadowCells.get(cell1).get(), shadowCell1);
+ assertEquals(cellsToShadowCells.get(cell2).get(), shadowCell2);
+ assertEquals(cellsToShadowCells.get(cell3).get(), shadowCell3);
+ assertFalse(cellsToShadowCells.get(cell4).isPresent());
+ }
+
}
diff --git a/hbase-common/src/main/java/org/apache/omid/transaction/CellUtils.java b/hbase-common/src/main/java/org/apache/omid/transaction/CellUtils.java
index 019ab74..98cbf4b 100644
--- a/hbase-common/src/main/java/org/apache/omid/transaction/CellUtils.java
+++ b/hbase-common/src/main/java/org/apache/omid/transaction/CellUtils.java
@@ -184,6 +184,24 @@ public final class CellUtils {
return qualLength;
}
+
+ /**
+ * Returns the qualifier length removing the shadow cell suffix and prefix. In case that que suffix is not found,
+ * just returns the length of the qualifier passed.
+ * @param qualifier the qualifier to remove the suffix from
+ * @param qualOffset the offset where the qualifier starts
+ * @param qualLength the qualifier length
+ * @return the qualifier length without the suffix
+ */
+ public static int qualifierOffsetFromShadowCellQualifier(byte[] qualifier, int qualOffset, int qualLength) {
+
+ if (startsWith(qualifier, qualOffset, qualLength, SHADOW_CELL_PREFIX)) {
+ return qualOffset + SHADOW_CELL_PREFIX.length;
+ }
+ return qualOffset;
+ }
+
+
/**
* Complement to matchingQualifier() methods in HBase's CellUtil.class
* @param left the cell to compare the qualifier
@@ -307,6 +325,7 @@ public final class CellUtils {
= new TreeMap<Cell, Optional<Cell>>(HBaseShims.cellComparatorInstance());
Map<CellId, Cell> cellIdToCellMap = new HashMap<CellId, Cell>();
+ Map<CellId, Cell> cellIdToSCCellMap = new HashMap<CellId, Cell>();
for (Cell cell : cells) {
if (!isShadowCell(cell)) {
CellId key = new CellId(cell, false);
@@ -328,7 +347,12 @@ public final class CellUtils {
}
} else {
cellIdToCellMap.put(key, cell);
- cellToShadowCellMap.put(cell, Optional.<Cell>absent());
+ Cell sc = cellIdToSCCellMap.get(key);
+ if (sc != null) {
+ cellToShadowCellMap.put(cell, Optional.of(sc));
+ } else {
+ cellToShadowCellMap.put(cell, Optional.<Cell>absent());
+ }
}
} else {
CellId key = new CellId(cell, true);
@@ -336,7 +360,7 @@ public final class CellUtils {
Cell originalCell = cellIdToCellMap.get(key);
cellToShadowCellMap.put(originalCell, Optional.of(cell));
} else {
- LOG.trace("Map does not contain key {}", key);
+ cellIdToSCCellMap.put(key, cell);
}
}
}
@@ -386,23 +410,29 @@ public final class CellUtils {
}
// Qualifier comparison
+ int qualifierLength = cell.getQualifierLength();
+ int qualifierOffset = cell.getQualifierOffset();
+ int otherQualifierLength = otherCell.getQualifierLength();
+ int otherQualifierOffset = otherCell.getQualifierOffset();
+
if (isShadowCell()) {
- int qualifierLength = qualifierLengthFromShadowCellQualifier(cell.getQualifierArray(),
+ qualifierLength = qualifierLengthFromShadowCellQualifier(cell.getQualifierArray(),
cell.getQualifierOffset(),
cell.getQualifierLength());
- int qualifierOffset = cell.getQualifierOffset();
- if (startsWith(cell.getQualifierArray(), cell.getQualifierOffset(),
- cell.getQualifierLength(), SHADOW_CELL_PREFIX)) {
- qualifierOffset = qualifierOffset + SHADOW_CELL_PREFIX.length;
- }
- if (!matchingQualifier(otherCell,
- cell.getQualifierArray(), qualifierOffset, qualifierLength)) {
- return false;
- }
- } else {
- if (!CellUtil.matchingQualifier(otherCell, cell)) {
- return false;
- }
+ qualifierOffset = qualifierOffsetFromShadowCellQualifier(cell.getQualifierArray(), cell.getQualifierOffset(),
+ cell.getQualifierLength());
+ }
+ if (otherCellId.isShadowCell()) {
+ otherQualifierLength = qualifierLengthFromShadowCellQualifier(otherCell.getQualifierArray(),
+ otherCell.getQualifierOffset(),
+ otherCell.getQualifierLength());
+ otherQualifierOffset = qualifierOffsetFromShadowCellQualifier(otherCell.getQualifierArray(), otherCell.getQualifierOffset(),
+ otherCell.getQualifierLength());
+ }
+
+ if (!Bytes.equals(cell.getQualifierArray(), qualifierOffset, qualifierLength,
+ otherCell.getQualifierArray(), otherQualifierOffset, otherQualifierLength)) {
+ return false;
}
// Timestamp comparison
@@ -426,6 +456,8 @@ public final class CellUtils {
qualifierOffset = qualifierOffset + SHADOW_CELL_PREFIX.length;
}
}
+ String a = Bytes.toString(cell.getQualifierArray(), qualifierOffset, qualifierLength);
+
hasher.putBytes(cell.getQualifierArray(),qualifierOffset , qualifierLength);
hasher.putLong(cell.getTimestamp());
return hasher.hash().asInt();
@@ -443,8 +475,8 @@ public final class CellUtils {
int qualifierLength = qualifierLengthFromShadowCellQualifier(cell.getQualifierArray(),
cell.getQualifierOffset(),
cell.getQualifierLength());
- helper.add("qualifier whithout shadow cell suffix",
- Bytes.toString(cell.getQualifierArray(), cell.getQualifierOffset() + 1, qualifierLength));
+ byte[] b = removeShadowCellSuffixPrefix(cell.getQualifierArray(), cell.getQualifierOffset(), cell.getQualifierLength());
+ helper.add("qualifier whithout shadow cell suffix", Bytes.toString(b));
}
helper.add("ts", cell.getTimestamp());
return helper.toString();
diff --git a/hbase-coprocessor/src/main/java/org/apache/omid/transaction/CompactorScanner.java b/hbase-coprocessor/src/main/java/org/apache/omid/transaction/CompactorScanner.java
index 4ba385d..ea14a8e 100644
--- a/hbase-coprocessor/src/main/java/org/apache/omid/transaction/CompactorScanner.java
+++ b/hbase-coprocessor/src/main/java/org/apache/omid/transaction/CompactorScanner.java
@@ -82,7 +82,6 @@ public class CompactorScanner implements InternalScanner {
@Override
public boolean next(List<Cell> results) throws IOException {
- //TODO YONIGO - why-1 we get exceptions
return next(results, -1);
}
@@ -166,7 +165,7 @@ public class CompactorScanner implements InternalScanner {
}
// Chomp current row worth values up to the limit
- if (currentRowWorthValues.size() <= limit) {
+ if (currentRowWorthValues.size() <= limit || limit == -1) {
result.addAll(currentRowWorthValues);
currentRowWorthValues.clear();
} else {
diff --git a/hbase-coprocessor/src/test/java/org/apache/omid/transaction/TestCompaction.java b/hbase-coprocessor/src/test/java/org/apache/omid/transaction/TestCompaction.java
index 197ef3f..8b77b7c 100644
--- a/hbase-coprocessor/src/test/java/org/apache/omid/transaction/TestCompaction.java
+++ b/hbase-coprocessor/src/test/java/org/apache/omid/transaction/TestCompaction.java
@@ -207,6 +207,67 @@ public class TestCompaction {
.build();
}
+
+ @Test
+ public void testShadowCellsAboveLWMSurviveCompaction() throws Exception {
+ String TEST_TABLE = "testShadowCellsAboveLWMSurviveCompaction";
+ createTableIfNotExists(TEST_TABLE, Bytes.toBytes(TEST_FAMILY));
+ TTable txTable = new TTable(connection, TEST_TABLE);
+
+ byte[] rowId = Bytes.toBytes("row");
+
+ // Create 3 transactions modifying the same cell in a particular row
+ HBaseTransaction tx1 = (HBaseTransaction) tm.begin();
+ Put put1 = new Put(rowId);
+ put1.addColumn(fam, qual, Bytes.toBytes("testValue 1"));
+ txTable.put(tx1, put1);
+ tm.commit(tx1);
+
+ HBaseTransaction tx2 = (HBaseTransaction) tm.begin();
+ Put put2 = new Put(rowId);
+ put2.addColumn(fam, qual, Bytes.toBytes("testValue 2"));
+ txTable.put(tx2, put2);
+ tm.commit(tx2);
+
+ HBaseTransaction tx3 = (HBaseTransaction) tm.begin();
+ Put put3 = new Put(rowId);
+ put3.addColumn(fam, qual, Bytes.toBytes("testValue 3"));
+ txTable.put(tx3, put3);
+ tm.commit(tx3);
+
+ // Before compaction, the three timestamped values for the cell should be there
+ TTableCellGetterAdapter getter = new TTableCellGetterAdapter(txTable);
+ assertTrue(CellUtils.hasCell(rowId, fam, qual, tx1.getStartTimestamp(), getter),
+ "Put cell of Tx1 should be there");
+ assertTrue(CellUtils.hasShadowCell(rowId, fam, qual, tx1.getStartTimestamp(), getter),
+ "Put shadow cell of Tx1 should be there");
+ assertTrue(CellUtils.hasCell(rowId, fam, qual, tx2.getStartTimestamp(), getter),
+ "Put cell of Tx2 cell should be there");
+ assertTrue(CellUtils.hasShadowCell(rowId, fam, qual, tx2.getStartTimestamp(), getter),
+ "Put shadow cell of Tx2 should be there");
+ assertTrue(CellUtils.hasCell(rowId, fam, qual, tx3.getStartTimestamp(), getter),
+ "Put cell of Tx3 cell should be there");
+ assertTrue(CellUtils.hasShadowCell(rowId, fam, qual, tx3.getStartTimestamp(), getter),
+ "Put shadow cell of Tx3 should be there");
+
+ // Compact
+ compactWithLWM(0, TEST_TABLE);
+
+ // After compaction, the three timestamped values for the cell should be there
+ assertTrue(CellUtils.hasCell(rowId, fam, qual, tx1.getStartTimestamp(), getter),
+ "Put cell of Tx1 should be there");
+ assertTrue(CellUtils.hasShadowCell(rowId, fam, qual, tx1.getStartTimestamp(), getter),
+ "Put shadow cell of Tx1 should be there");
+ assertTrue(CellUtils.hasCell(rowId, fam, qual, tx2.getStartTimestamp(), getter),
+ "Put cell of Tx2 cell should be there");
+ assertTrue(CellUtils.hasShadowCell(rowId, fam, qual, tx2.getStartTimestamp(), getter),
+ "Put shadow cell of Tx2 should be there");
+ assertTrue(CellUtils.hasCell(rowId, fam, qual, tx3.getStartTimestamp(), getter),
+ "Put cell of Tx3 cell should be there");
+ assertTrue(CellUtils.hasShadowCell(rowId, fam, qual, tx3.getStartTimestamp(), getter),
+ "Put shadow cell of Tx3 should be there");
+ }
+
@Test(timeOut = 60_000)
public void testStandardTXsWithShadowCellsAndWithSTBelowAndAboveLWMArePresevedAfterCompaction() throws Throwable {
String TEST_TABLE = "testStandardTXsWithShadowCellsAndWithSTBelowAndAboveLWMArePresevedAfterCompaction";