accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [accumulo] branch master updated: Fix #1281 Update TabletTime.maxMetatdataTime (#1304)
Date Thu, 01 Aug 2019 20:56:47 GMT
This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/master by this push:
     new 35758a0  Fix #1281 Update TabletTime.maxMetatdataTime (#1304)
35758a0 is described below

commit 35758a09ae2b83a796bbd3b0a2a3e81a452e156b
Author: hkeebler <49656678+hkeebler@users.noreply.github.com>
AuthorDate: Thu Aug 1 16:56:41 2019 -0400

    Fix #1281 Update TabletTime.maxMetatdataTime (#1304)
    
    replace string processing with MetadataTime objects
---
 .../core/metadata/schema/MetadataTime.java         | 15 ++++--
 .../core/metadata/schema/MetadataTimeTest.java     | 61 ++++++++++++++++------
 .../apache/accumulo/server/tablets/TabletTime.java | 28 ++--------
 .../accumulo/server/tablets/TabletTimeTest.java    | 60 +++++++--------------
 .../apache/accumulo/master/TabletGroupWatcher.java | 10 ++--
 5 files changed, 86 insertions(+), 88 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataTime.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataTime.java
index ed06a29..77c7ec2 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataTime.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataTime.java
@@ -23,7 +23,7 @@ import org.apache.accumulo.core.client.admin.TimeType;
 /**
  * Immutable metadata time object
  */
-public final class MetadataTime {
+public final class MetadataTime implements Comparable<MetadataTime> {
   private final long time;
   private final TimeType type;
 
@@ -43,7 +43,7 @@ public final class MetadataTime {
   public static MetadataTime parse(String timestr) throws IllegalArgumentException {
 
     if (timestr != null && timestr.length() > 1) {
-      return new MetadataTime(Long.parseLong(timestr.substring(1)), valueOf(timestr.charAt(0)));
+      return new MetadataTime(Long.parseLong(timestr.substring(1)), getType(timestr.charAt(0)));
     } else
       throw new IllegalArgumentException("Unknown metadata time value " + timestr);
   }
@@ -55,7 +55,7 @@ public final class MetadataTime {
    *          character M or L otherwise exception thrown
    * @return a TimeType {@link TimeType} represented by code.
    */
-  public static TimeType valueOf(char code) {
+  public static TimeType getType(char code) {
     switch (code) {
       case 'M':
         return TimeType.MILLIS;
@@ -110,4 +110,13 @@ public final class MetadataTime {
     return Objects.hash(time, type);
   }
 
+  @Override
+  public int compareTo(MetadataTime mtime) {
+    if (this.type.equals(mtime.getType()))
+      return Long.compare(this.time, mtime.getTime());
+    else
+      throw new IllegalArgumentException(
+          "Cannot compare different time types: " + this + " and " + mtime);
+  }
+
 }
diff --git a/core/src/test/java/org/apache/accumulo/core/metadata/schema/MetadataTimeTest.java
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/MetadataTimeTest.java
index fbe706c..f71aaf2 100644
--- a/core/src/test/java/org/apache/accumulo/core/metadata/schema/MetadataTimeTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/metadata/schema/MetadataTimeTest.java
@@ -18,12 +18,18 @@ package org.apache.accumulo.core.metadata.schema;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
 
 import org.apache.accumulo.core.client.admin.TimeType;
 import org.junit.Test;
 
 public class MetadataTimeTest {
 
+  private static final MetadataTime m1234 = new MetadataTime(1234, TimeType.MILLIS);
+  private static final MetadataTime m5678 = new MetadataTime(5678, TimeType.MILLIS);
+  private static final MetadataTime l1234 = new MetadataTime(1234, TimeType.LOGICAL);
+  private static final MetadataTime l5678 = new MetadataTime(5678, TimeType.LOGICAL);
+
   @Test(expected = IllegalArgumentException.class)
   public void testGetInstance_InvalidType() {
     MetadataTime.parse("X1234");
@@ -41,39 +47,37 @@ public class MetadataTimeTest {
 
   @Test
   public void testGetInstance_Millis() {
-    MetadataTime mTime = new MetadataTime(1234, TimeType.MILLIS);
-    assertEquals(1234, mTime.getTime());
-    assertEquals(TimeType.MILLIS, mTime.getType());
+    assertEquals(1234, m1234.getTime());
+    assertEquals(TimeType.MILLIS, m1234.getType());
   }
 
   @Test
   public void testGetInstance_Logical() {
-    MetadataTime mTime = new MetadataTime(1234, TimeType.LOGICAL);
-    assertEquals(1234, mTime.getTime());
-    assertEquals(TimeType.LOGICAL, mTime.getType());
+    assertEquals(1234, l1234.getTime());
+    assertEquals(TimeType.LOGICAL, l1234.getType());
 
   }
 
   @Test
   public void testEquality() {
-    assertEquals(new MetadataTime(21, TimeType.MILLIS), MetadataTime.parse("M21"));
-    assertNotEquals(new MetadataTime(21, TimeType.MILLIS), MetadataTime.parse("L21"));
-    assertNotEquals(new MetadataTime(21, TimeType.LOGICAL), new MetadataTime(44, TimeType.LOGICAL));
+    assertEquals(m1234, new MetadataTime(1234, TimeType.MILLIS));
+    assertNotEquals(m1234, l1234);
+    assertNotEquals(l1234, l5678);
   }
 
   @Test
   public void testValueOfM() {
-    assertEquals(TimeType.MILLIS, MetadataTime.valueOf('M'));
+    assertEquals(TimeType.MILLIS, MetadataTime.getType('M'));
   }
 
   @Test
   public void testValueOfL() {
-    assertEquals(TimeType.LOGICAL, MetadataTime.valueOf('L'));
+    assertEquals(TimeType.LOGICAL, MetadataTime.getType('L'));
   }
 
   @Test(expected = IllegalArgumentException.class)
   public void testValueOfOtherChar() {
-    MetadataTime.valueOf('x');
+    MetadataTime.getType('x');
   }
 
   @Test
@@ -84,14 +88,12 @@ public class MetadataTimeTest {
 
   @Test
   public void testgetCodeforMillis() {
-    MetadataTime mTime = new MetadataTime(0, TimeType.MILLIS);
-    assertEquals('M', mTime.getCode());
+    assertEquals('M', m1234.getCode());
   }
 
   @Test
   public void testgetCodeforLogical() {
-    MetadataTime mTime = new MetadataTime(0, TimeType.LOGICAL);
-    assertEquals('L', mTime.getCode());
+    assertEquals('L', l1234.getCode());
   }
 
   @Test
@@ -100,4 +102,31 @@ public class MetadataTimeTest {
     assertEquals("L45678", new MetadataTime(45678, TimeType.LOGICAL).encode());
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testCompareTypesDiffer1() {
+    m1234.compareTo(l1234);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCompareTypesDiffer2() {
+    l1234.compareTo(m1234);
+  }
+
+  @Test
+  public void testCompareSame() {
+    assertTrue(m1234.compareTo(m1234) == 0);
+    assertTrue(l1234.compareTo(l1234) == 0);
+  }
+
+  @Test
+  public void testCompare1() {
+    assertTrue(m1234.compareTo(m5678) < 0);
+    assertTrue(l1234.compareTo(l5678) < 0);
+  }
+
+  @Test
+  public void testCompare2() {
+    assertTrue(m5678.compareTo(m1234) > 0);
+    assertTrue(l5678.compareTo(l1234) > 0);
+  }
 }
diff --git a/server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java
b/server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java
index 28eddef..23cbce7 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/tablets/TabletTime.java
@@ -54,30 +54,12 @@ public abstract class TabletTime {
       throw new IllegalArgumentException("Time type unknown : " + metadataTime);
   }
 
-  public static String maxMetadataTime(String mv1, String mv2) {
-    if (mv1 == null && mv2 == null) {
-      return null;
-    }
-    // the parse is used to validate the string
-    if (mv1 == null) {
-      return MetadataTime.parse(mv2).encode();
-    }
-
-    if (mv2 == null) {
-      return MetadataTime.parse(mv1).encode();
-    }
-
-    MetadataTime mv1Time = MetadataTime.parse(mv1);
-    MetadataTime mv2Time = MetadataTime.parse(mv2);
-
-    if (mv1Time.getType() != mv2Time.getType())
-      throw new IllegalArgumentException("Time types differ " + mv1 + " " + mv2);
-
-    if (mv1Time.getTime() < mv2Time.getTime())
-      return mv2;
-    else
-      return mv1;
+  public static MetadataTime maxMetadataTime(MetadataTime mv1, MetadataTime mv2) {
+    // null value will sort lower
+    if (mv1 == null || mv2 == null)
+      return mv1 == null ? (mv2 == null ? null : mv2) : mv1;
 
+    return mv1.compareTo(mv2) < 0 ? mv2 : mv1;
   }
 
   static class MillisTime extends TabletTime {
diff --git a/server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java
b/server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java
index 2ea0295..60155ab 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/tablets/TabletTimeTest.java
@@ -33,6 +33,10 @@ import org.junit.Test;
 public class TabletTimeTest {
   private static final long TIME = 1234L;
   private MillisTime mtime;
+  private static final MetadataTime m1234 = new MetadataTime(1234, TimeType.MILLIS);
+  private static final MetadataTime m5678 = new MetadataTime(5678, TimeType.MILLIS);
+  private static final MetadataTime l1234 = new MetadataTime(1234, TimeType.LOGICAL);
+  private static final MetadataTime l5678 = new MetadataTime(5678, TimeType.LOGICAL);
 
   @Before
   public void setUp() {
@@ -65,72 +69,44 @@ public class TabletTimeTest {
 
   @Test
   public void testMaxMetadataTime_Logical() {
-    assertEquals("L5678", TabletTime.maxMetadataTime("L1234", "L5678"));
-    assertEquals("L5678", TabletTime.maxMetadataTime("L5678", "L1234"));
-    assertEquals("L5678", TabletTime.maxMetadataTime("L5678", "L5678"));
+    assertEquals(l5678, TabletTime.maxMetadataTime(l1234, l5678));
+    assertEquals(l5678, TabletTime.maxMetadataTime(l5678, l1234));
+    assertEquals(l5678, TabletTime.maxMetadataTime(l5678, l5678));
   }
 
   @Test
   public void testMaxMetadataTime_Millis() {
-    assertEquals("M5678", TabletTime.maxMetadataTime("M1234", "M5678"));
-    assertEquals("M5678", TabletTime.maxMetadataTime("M5678", "M1234"));
-    assertEquals("M5678", TabletTime.maxMetadataTime("M5678", "M5678"));
+    assertEquals(m5678, TabletTime.maxMetadataTime(m1234, m5678));
+    assertEquals(m5678, TabletTime.maxMetadataTime(m5678, m1234));
+    assertEquals(m5678, TabletTime.maxMetadataTime(m5678, m5678));
   }
 
   @Test
   public void testMaxMetadataTime_Null1() {
-    assertEquals("L5678", TabletTime.maxMetadataTime(null, "L5678"));
-    assertEquals("M5678", TabletTime.maxMetadataTime(null, "M5678"));
+    assertEquals(l5678, TabletTime.maxMetadataTime(null, l5678));
+    assertEquals(m5678, TabletTime.maxMetadataTime(null, m5678));
   }
 
   @Test
   public void testMaxMetadataTime_Null2() {
-    assertEquals("L5678", TabletTime.maxMetadataTime("L5678", null));
-    assertEquals("M5678", TabletTime.maxMetadataTime("M5678", null));
+    assertEquals(l5678, TabletTime.maxMetadataTime(l5678, null));
+    assertEquals(m5678, TabletTime.maxMetadataTime(m5678, null));
   }
 
   @Test
   public void testMaxMetadataTime_Null3() {
-    assertNull(TabletTime.maxMetadataTime(null, null));
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testMaxMetadataTime_Null1_Invalid() {
-    TabletTime.maxMetadataTime(null, "X5678");
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testMaxMetadataTime_Null2_Invalid() {
-    TabletTime.maxMetadataTime("X5678", null);
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testMaxMetadataTime_Invalid1() {
-    TabletTime.maxMetadataTime("X1234", "L5678");
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testMaxMetadataTime_Invalid2() {
-    TabletTime.maxMetadataTime("L1234", "X5678");
+    MetadataTime nullTime = null;
+    assertNull(TabletTime.maxMetadataTime(nullTime, nullTime));
   }
 
   @Test(expected = IllegalArgumentException.class)
   public void testMaxMetadataTime_DifferentTypes1() {
-    TabletTime.maxMetadataTime("L1234", "M5678");
+    TabletTime.maxMetadataTime(l1234, m5678);
   }
 
   @Test(expected = IllegalArgumentException.class)
   public void testMaxMetadataTime_DifferentTypes2() {
-    TabletTime.maxMetadataTime("X1234", "Y5678");
+    TabletTime.maxMetadataTime(m1234, l5678);
   }
 
-  @Test(expected = NumberFormatException.class)
-  public void testMaxMetadataTime_ParseFailure1() {
-    TabletTime.maxMetadataTime("L1234", "LABCD");
-  }
-
-  @Test(expected = NumberFormatException.class)
-  public void testMaxMetadataTime_ParseFailure2() {
-    TabletTime.maxMetadataTime("LABCD", "L5678");
-  }
 }
diff --git a/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
b/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
index 8060885..64eb672 100644
--- a/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
+++ b/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
@@ -708,7 +708,7 @@ abstract class TabletGroupWatcher extends Daemon {
       TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.fetch(scanner);
       scanner.fetchColumnFamily(DataFileColumnFamily.NAME);
       Mutation m = new Mutation(stopRow);
-      String maxLogicalTime = null;
+      MetadataTime maxLogicalTime = null;
       for (Entry<Key,Value> entry : scanner) {
         Key key = entry.getKey();
         Value value = entry.getValue();
@@ -720,7 +720,8 @@ abstract class TabletGroupWatcher extends Daemon {
           Master.log.debug("prevRow entry for lowest tablet is {}", value);
           firstPrevRowValue = new Value(value);
         } else if (TabletsSection.ServerColumnFamily.TIME_COLUMN.hasColumns(key)) {
-          maxLogicalTime = TabletTime.maxMetadataTime(maxLogicalTime, value.toString());
+          maxLogicalTime =
+              TabletTime.maxMetadataTime(maxLogicalTime, MetadataTime.parse(value.toString()));
         } else if (TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.hasColumns(key)) {
           bw.addMutation(ServerAmpleImpl.createDeleteMutation(master.getContext(),
               range.getTableId(), entry.getValue().toString()));
@@ -734,12 +735,13 @@ abstract class TabletGroupWatcher extends Daemon {
       TabletsSection.ServerColumnFamily.TIME_COLUMN.fetch(scanner);
       for (Entry<Key,Value> entry : scanner) {
         if (TabletsSection.ServerColumnFamily.TIME_COLUMN.hasColumns(entry.getKey())) {
-          maxLogicalTime = TabletTime.maxMetadataTime(maxLogicalTime, entry.getValue().toString());
+          maxLogicalTime = TabletTime.maxMetadataTime(maxLogicalTime,
+              MetadataTime.parse(entry.getValue().toString()));
         }
       }
 
       if (maxLogicalTime != null)
-        TabletsSection.ServerColumnFamily.TIME_COLUMN.put(m, new Value(maxLogicalTime.getBytes()));
+        TabletsSection.ServerColumnFamily.TIME_COLUMN.put(m, new Value(maxLogicalTime.encode()));
 
       if (!m.getUpdates().isEmpty()) {
         bw.addMutation(m);


Mime
View raw message