diff --git a/CHANGES.txt b/CHANGES.txt index 78c7ba06518..2ad10158fad 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -388,6 +388,8 @@ Release 0.21.0 - Unreleased HBASE-2732 TestZooKeeper was broken, HBASE-2691 showed it HBASE-2670 Provide atomicity for readers even when new insert has same timestamp as current row. + HBASE-2733 Replacement of LATEST_TIMESTAMP with real timestamp was broken + by HBASE-2353. IMPROVEMENTS HBASE-1760 Cleanup TODOs in HTable diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 06e022cb410..61b8197f2e6 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -1327,7 +1327,7 @@ public class HRegion implements HeapSize { // , Writable{ // bunch up all edits across all column families into a // single WALEdit. WALEdit walEdit = new WALEdit(); - addFamilyMapToWALEdit(familyMap, byteNow, walEdit); + addFamilyMapToWALEdit(familyMap, walEdit); this.log.append(regionInfo, regionInfo.getTableDesc().getName(), walEdit, now); } @@ -1520,9 +1520,18 @@ public class HRegion implements HeapSize { // , Writable{ } // We've now grabbed as many puts off the list as we can assert numReadyToWrite > 0; + + // ------------------------------------ + // STEP 2. Update any LATEST_TIMESTAMP timestamps + // ---------------------------------- + for (int i = firstIndex; i < lastIndexExclusive; i++) { + updateKVTimestamps( + batchOp.operations[i].getFirst().getFamilyMap().values(), + byteNow); + } // ------------------------------------ - // STEP 2. Write to WAL + // STEP 3. Write to WAL // ---------------------------------- WALEdit walEdit = new WALEdit(); for (int i = firstIndex; i < lastIndexExclusive; i++) { @@ -1531,7 +1540,7 @@ public class HRegion implements HeapSize { // , Writable{ Put p = batchOp.operations[i].getFirst(); if (!p.getWriteToWAL()) continue; - addFamilyMapToWALEdit(p.getFamilyMap(), byteNow, walEdit); + addFamilyMapToWALEdit(p.getFamilyMap(), walEdit); } // Append the edit to WAL @@ -1539,7 +1548,7 @@ public class HRegion implements HeapSize { // , Writable{ walEdit, now); // ------------------------------------ - // STEP 3. Write back to memstore + // STEP 4. Write back to memstore // ---------------------------------- long addedSize = 0; for (int i = firstIndex; i < lastIndexExclusive; i++) { @@ -1636,21 +1645,17 @@ public class HRegion implements HeapSize { // , Writable{ /** - * Checks if any stamps is Long.MAX_VALUE. If so, sets them to now. - *

- * This acts to replace {@link HConstants#LATEST_TIMESTAMP} with {@code now}. - * @param keys - * @param now - * @return true when updating the time stamp completed. + * Replaces any KV timestamps set to {@link HConstants#LATEST_TIMESTAMP} + * with the provided current timestamp. */ - private boolean updateKeys(final List keys, final byte[] now) { - if (keys == null || keys.isEmpty()) { - return false; + private void updateKVTimestamps( + final Iterable> keyLists, final byte[] now) { + for (List keys: keyLists) { + if (keys == null) continue; + for (KeyValue key : keys) { + key.updateLatestStamp(now); + } } - for (KeyValue key : keys) { - key.updateLatestStamp(now); - } - return true; } // /* @@ -1754,7 +1759,7 @@ public class HRegion implements HeapSize { // , Writable{ this.updatesLock.readLock().lock(); try { checkFamilies(familyMap.keySet()); - + updateKVTimestamps(familyMap.values(), byteNow); // write/sync to WAL should happen before we touch memstore. // // If order is reversed, i.e. we write to memstore first, and @@ -1762,7 +1767,7 @@ public class HRegion implements HeapSize { // , Writable{ // will contain uncommitted transactions. if (writeToWAL) { WALEdit walEdit = new WALEdit(); - addFamilyMapToWALEdit(familyMap, byteNow, walEdit); + addFamilyMapToWALEdit(familyMap, walEdit); this.log.append(regionInfo, regionInfo.getTableDesc().getName(), walEdit, now); } @@ -1822,22 +1827,15 @@ public class HRegion implements HeapSize { // , Writable{ /** * Append the given map of family->edits to a WALEdit data structure. - * Also updates the timestamps of the edits where they have not - * been specified by the user. This does not write to the HLog itself. + * This does not write to the HLog itself. * @param familyMap map of family->edits - * @param byteNow timestamp to use when unspecified * @param walEdit the destination entry to append into */ private void addFamilyMapToWALEdit(Map> familyMap, - byte[] byteNow, WALEdit walEdit) { + WALEdit walEdit) { for (List edits : familyMap.values()) { - // update timestamp on keys if required. - if (updateKeys(edits, byteNow)) { - // bunch up all edits across all column families into a - // single WALEdit. - for (KeyValue kv : edits) { - walEdit.add(kv); - } + for (KeyValue kv : edits) { + walEdit.add(kv); } } } diff --git a/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java index 1c1cd4be200..48b8ac08353 100644 --- a/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java +++ b/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java @@ -345,6 +345,7 @@ public class TestHRegion extends HBaseTestCase { byte[] val = Bytes.toBytes("val"); initHRegion(b, getName(), cf); + HLog.getSyncOps(); // clear counter from prior tests assertEquals(0, HLog.getSyncOps()); LOG.info("First a batch put with all valid puts"); @@ -833,6 +834,53 @@ public class TestHRegion extends HBaseTestCase { result = region.get(get, null); assertEquals(0, result.size()); } + + /** + * Tests that the special LATEST_TIMESTAMP option for puts gets + * replaced by the actual timestamp + */ + public void testPutWithLatestTS() throws IOException { + byte [] tableName = Bytes.toBytes("testtable"); + byte [] fam = Bytes.toBytes("info"); + byte [][] families = {fam}; + String method = this.getName(); + initHRegion(tableName, method, families); + + byte [] row = Bytes.toBytes("row1"); + // column names + byte [] qual = Bytes.toBytes("qual"); + + // add data with LATEST_TIMESTAMP, put without WAL + Put put = new Put(row); + put.add(fam, qual, HConstants.LATEST_TIMESTAMP, Bytes.toBytes("value")); + region.put(put, false); + + // Make sure it shows up with an actual timestamp + Get get = new Get(row).addColumn(fam, qual); + Result result = region.get(get, null); + assertEquals(1, result.size()); + KeyValue kv = result.raw()[0]; + LOG.info("Got: " + kv); + assertTrue("LATEST_TIMESTAMP was not replaced with real timestamp", + kv.getTimestamp() != HConstants.LATEST_TIMESTAMP); + + // Check same with WAL enabled (historically these took different + // code paths, so check both) + row = Bytes.toBytes("row2"); + put = new Put(row); + put.add(fam, qual, HConstants.LATEST_TIMESTAMP, Bytes.toBytes("value")); + region.put(put, true); + + // Make sure it shows up with an actual timestamp + get = new Get(row).addColumn(fam, qual); + result = region.get(get, null); + assertEquals(1, result.size()); + kv = result.raw()[0]; + LOG.info("Got: " + kv); + assertTrue("LATEST_TIMESTAMP was not replaced with real timestamp", + kv.getTimestamp() != HConstants.LATEST_TIMESTAMP); + + } public void testScanner_DeleteOneFamilyNotAnother() throws IOException { byte [] tableName = Bytes.toBytes("test_table");