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/13 09:26:45 UTC
[incubator-omid] branch 1.0.1 updated (01ce12a -> 14b5dec)
This is an automated email from the ASF dual-hosted git repository.
yonigo pushed a change to branch 1.0.1
in repository https://gitbox.apache.org/repos/asf/incubator-omid.git.
discard 01ce12a [OMID-135] - Fix compaction bug that deletes shadow cells above LWM
new 14b5dec [OMID-135] - Fix compaction bug that deletes shadow cells above LWM
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (01ce12a)
\
N -- N -- N refs/heads/1.0.1 (14b5dec)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../src/main/java/org/apache/omid/transaction/CellUtils.java | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
[incubator-omid] 01/01: [OMID-135] - Fix compaction bug that
deletes shadow cells above LWM
Posted by yo...@apache.org.
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 14b5decdd26749226621be656ff032aa175371d2
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 | 76 ++++++++++++++++------
.../apache/omid/transaction/CompactorScanner.java | 3 +-
.../apache/omid/transaction/TestCompaction.java | 61 +++++++++++++++++
4 files changed, 170 insertions(+), 23 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..672659b 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);
@@ -319,24 +338,31 @@ public final class CellUtils {
} else {
if (cell.getSequenceId() > storedCell.getSequenceId()) { // Swap values
Optional<Cell> previousValue = cellToShadowCellMap.remove(storedCell);
- Preconditions.checkNotNull(previousValue, "Should contain an Optional<Cell> value");
cellIdToCellMap.put(key, cell);
- cellToShadowCellMap.put(cell, previousValue);
+ if (previousValue != null) {
+ cellToShadowCellMap.put(cell, previousValue);
+ }
} else {
LOG.warn("Cell {} with an earlier MVCC found. Ignoring...", cell);
}
}
} 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);
- if (cellIdToCellMap.containsKey(key)) {
+ Cell savedCell = cellIdToCellMap.get(key);
+ if (savedCell != null) {
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 +412,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 +458,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 +477,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";