YARN-3530. ATS throws exception on trying to filter results without

otherinfo. Contributed by zhijie shen

hadoop-yarn-project/CHANGES.txt

@@ -271,6 +271,9 @@ Release 2.8.0 - UNRELEASED
     YARN-3464. Race condition in LocalizerRunner kills localizer before 
     localizing all resources. (Zhihai Xu via kasha)
 
+    YARN-3530. ATS throws exception on trying to filter results without otherinfo.
+    (zhijie shen via xgong)
+
 Release 2.7.1 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/LeveldbTimelineStore.java

@@ -357,6 +357,9 @@ public class LeveldbTimelineStore extends AbstractService
       iterator = new LeveldbIterator(db);
       iterator.seek(prefix);
 
+      if (fields == null) {
+        fields = EnumSet.allOf(Field.class);
+      }
       return getEntity(entityId, entityType, revStartTime, fields, iterator,
           prefix, prefix.length);
     } catch(DBException e) {
@@ -373,10 +376,6 @@ public class LeveldbTimelineStore extends AbstractService
   private static TimelineEntity getEntity(String entityId, String entityType,
       Long startTime, EnumSet<Field> fields, LeveldbIterator iterator,
       byte[] prefix, int prefixlen) throws IOException {
-    if (fields == null) {
-      fields = EnumSet.allOf(Field.class);
-    }
-
     TimelineEntity entity = new TimelineEntity();
     boolean events = false;
     boolean lastEvent = false;
@@ -590,6 +589,25 @@ public class LeveldbTimelineStore extends AbstractService
       String entityType, Long limit, Long starttime, Long endtime,
       String fromId, Long fromTs, Collection<NameValuePair> secondaryFilters,
       EnumSet<Field> fields, CheckAcl checkAcl) throws IOException {
+    // Even if other info and primary filter fields are not included, we
+    // still need to load them to match secondary filters when they are
+    // non-empty
+    if (fields == null) {
+      fields = EnumSet.allOf(Field.class);
+    }
+    boolean addPrimaryFilters = false;
+    boolean addOtherInfo = false;
+    if (secondaryFilters != null && secondaryFilters.size() > 0) {
+      if (!fields.contains(Field.PRIMARY_FILTERS)) {
+        fields.add(Field.PRIMARY_FILTERS);
+        addPrimaryFilters = true;
+      }
+      if (!fields.contains(Field.OTHER_INFO)) {
+        fields.add(Field.OTHER_INFO);
+        addOtherInfo = true;
+      }
+    }
+
     LeveldbIterator iterator = null;
     try {
       KeyBuilder kb = KeyBuilder.newInstance().add(base).add(entityType);
@@ -690,6 +708,14 @@ public class LeveldbTimelineStore extends AbstractService
             entity.setDomainId(DEFAULT_DOMAIN_ID);
           }
           if (checkAcl == null || checkAcl.check(entity)) {
+            // Remove primary filter and other info if they are added for
+            // matching secondary filters
+            if (addPrimaryFilters) {
+              entity.setPrimaryFilters(null);
+            }
+            if (addOtherInfo) {
+              entity.setOtherInfo(null);
+            }
             entities.addEntity(entity);
           }
         }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/TimelineStoreTestUtils.java

@@ -28,6 +28,7 @@ import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -487,6 +488,13 @@ public class TimelineStoreTestUtils {
         primaryFilter, secondaryFilters, null, null).getEntities();
   }
 
+  protected List<TimelineEntity> getEntitiesWithFilters(String entityType,
+      NameValuePair primaryFilter, Collection<NameValuePair> secondaryFilters,
+      EnumSet<Field> fields) throws IOException {
+    return store.getEntities(entityType, null, null, null, null, null,
+        primaryFilter, secondaryFilters, fields, null).getEntities();
+  }
+
   protected List<TimelineEntity> getEntities(String entityType, Long limit,
       Long windowStart, Long windowEnd, NameValuePair primaryFilter,
       EnumSet<Field> fields) throws IOException {
@@ -751,38 +759,73 @@ public class TimelineStoreTestUtils {
   }
 
   public void testGetEntitiesWithSecondaryFilters() throws IOException {
-    // test using secondary filter
-    List<TimelineEntity> entities = getEntitiesWithFilters("type_1", null,
-        goodTestingFilters);
-    assertEquals(3, entities.size());
-    verifyEntityInfo(entityId1, entityType1, events1, EMPTY_REL_ENTITIES,
-        primaryFilters, otherInfo, entities.get(0), domainId1);
-    verifyEntityInfo(entityId1b, entityType1, events1, EMPTY_REL_ENTITIES,
-        primaryFilters, otherInfo, entities.get(1), domainId1);
-    verifyEntityInfo(entityId6, entityType1, EMPTY_EVENTS, EMPTY_REL_ENTITIES,
-        primaryFilters, otherInfo, entities.get(2), domainId2);
-
-    entities = getEntitiesWithFilters("type_1", userFilter, goodTestingFilters);
-    assertEquals(3, entities.size());
-    verifyEntityInfo(entityId1, entityType1, events1, EMPTY_REL_ENTITIES,
-        primaryFilters, otherInfo, entities.get(0), domainId1);
-    verifyEntityInfo(entityId1b, entityType1, events1, EMPTY_REL_ENTITIES,
-        primaryFilters, otherInfo, entities.get(1), domainId1);
-    verifyEntityInfo(entityId6, entityType1, EMPTY_EVENTS, EMPTY_REL_ENTITIES,
-        primaryFilters, otherInfo, entities.get(2), domainId2);
+    for (int i = 0; i < 4; ++i) {
+      // Verify the secondary filter works both other info is included or not.
+      EnumSet<Field> fields = null;
+      if (i == 1) {
+        fields = EnumSet.noneOf(Field.class);
+      } else if (i == 2) {
+        fields = EnumSet.of(Field.PRIMARY_FILTERS);
+      } else if (i == 3) {
+        fields = EnumSet.of(Field.OTHER_INFO);
+      }
+      // test using secondary filter
+      List<TimelineEntity> entities = getEntitiesWithFilters("type_1", null,
+          goodTestingFilters, fields);
+      assertEquals(3, entities.size());
+      verifyEntityInfo(entityId1, entityType1,
+          (i == 0 ? events1 : null),
+          (i == 0 ? EMPTY_REL_ENTITIES : null),
+          (i == 0 || i == 2 ? primaryFilters : null),
+          (i == 0 || i == 3 ? otherInfo : null), entities.get(0), domainId1);
+      verifyEntityInfo(entityId1b, entityType1,
+          (i == 0 ? events1 : null),
+          (i == 0 ? EMPTY_REL_ENTITIES : null),
+          (i == 0 || i == 2 ? primaryFilters : null),
+          (i == 0 || i == 3 ? otherInfo : null), entities.get(1), domainId1);
+      verifyEntityInfo(entityId6, entityType1,
+          (i == 0 ? EMPTY_EVENTS : null),
+          (i == 0 ? EMPTY_REL_ENTITIES : null),
+          (i == 0 || i == 2 ? primaryFilters : null),
+          (i == 0 || i == 3 ? otherInfo : null), entities.get(2), domainId2);
+
+      entities =
+          getEntitiesWithFilters("type_1", userFilter, goodTestingFilters, fields);
+      assertEquals(3, entities.size());
+      if (i == 0) {
+        verifyEntityInfo(entityId1, entityType1,
+            (i == 0 ? events1 : null),
+            (i == 0 ? EMPTY_REL_ENTITIES : null),
+            (i == 0 || i == 2 ? primaryFilters : null),
+            (i == 0 || i == 3 ? otherInfo : null), entities.get(0), domainId1);
+        verifyEntityInfo(entityId1b, entityType1,
+            (i == 0 ? events1 : null),
+            (i == 0 ? EMPTY_REL_ENTITIES : null),
+            (i == 0 || i == 2 ? primaryFilters : null),
+            (i == 0 || i == 3 ? otherInfo : null), entities.get(1), domainId1);
+        verifyEntityInfo(entityId6, entityType1,
+            (i == 0 ? EMPTY_EVENTS : null),
+            (i == 0 ? EMPTY_REL_ENTITIES : null),
+            (i == 0 || i == 2 ? primaryFilters : null),
+            (i == 0 || i == 3 ? otherInfo : null), entities.get(2), domainId2);
+      }
 
-    entities = getEntitiesWithFilters("type_1", null,
-        Collections.singleton(new NameValuePair("user", "none")));
-    assertEquals(0, entities.size());
+      entities = getEntitiesWithFilters("type_1", null,
+          Collections.singleton(new NameValuePair("user", "none")), fields);
+      assertEquals(0, entities.size());
 
-    entities = getEntitiesWithFilters("type_1", null, badTestingFilters);
-    assertEquals(0, entities.size());
+      entities =
+          getEntitiesWithFilters("type_1", null, badTestingFilters, fields);
+      assertEquals(0, entities.size());
 
-    entities = getEntitiesWithFilters("type_1", userFilter, badTestingFilters);
-    assertEquals(0, entities.size());
+      entities =
+          getEntitiesWithFilters("type_1", userFilter, badTestingFilters, fields);
+      assertEquals(0, entities.size());
 
-    entities = getEntitiesWithFilters("type_5", null, badTestingFilters);
-    assertEquals(0, entities.size());
+      entities =
+          getEntitiesWithFilters("type_5", null, badTestingFilters, fields);
+      assertEquals(0, entities.size());
+    }
   }
 
   public void testGetEvents() throws IOException {