Skip to content

Commit

Permalink
[SPARK-31584][WEBUI] Fix NullPointerException when parsing event log …
Browse files Browse the repository at this point in the history
…with InMemoryStore

### What changes were proposed in this pull request?
#27716 introduced parent index for InMemoryStore. When the method "deleteParentIndex(Object key)" in InMemoryStore.java is called and the key is not contained in "NaturalKeys v",  A java.lang.NullPointerException will be thrown. This patch fixed the issue by updating the if condition.

### Why are the changes needed?
Fixed a minor bug.

### Does this PR introduce any user-facing change?
No.

### How was this patch tested?
Added a unit test for deleteParentIndex.

Closes #28378 from baohe-zhang/SPARK-31584.

Authored-by: Baohe Zhang <baohe.zhang@verizonmedia.com>
Signed-off-by: Gengliang Wang <gengliang.wang@databricks.com>
(cherry picked from commit 3808014)
Signed-off-by: Gengliang Wang <gengliang.wang@databricks.com>
  • Loading branch information
Baohe Zhang authored and gengliangwang committed Apr 29, 2020
1 parent da8c7b8 commit 0058212
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public boolean delete(Object key, T value) {
private void deleteParentIndex(Object key) {
if (hasNaturalParentIndex) {
for (NaturalKeys v : parentToChildrenMap.values()) {
if (v.remove(asKey(key))) {
if (v.remove(asKey(key)) != null) {
// `v` can be empty after removing the natural key and we can remove it from
// `parentToChildrenMap`. However, `parentToChildrenMap` is a ConcurrentMap and such
// checking and deleting can be slow.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.util.kvstore;

public class CustomType2 {

@KVIndex(parent = "parentId")
public String key;

@KVIndex("id")
public String id;

@KVIndex("parentId")
public String parentId;

@Override
public boolean equals(Object o) {
if (o instanceof CustomType2) {
CustomType2 other = (CustomType2) o;
return id.equals(other.id) && parentId.equals(other.parentId);
}
return false;
}

@Override
public int hashCode() {
return id.hashCode() ^ parentId.hashCode();
}

@Override
public String toString() {
return "CustomType2[key=" + key + ",id=" + id + ",parentId=" + parentId;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -210,4 +210,46 @@ public void testBasicIteration() throws Exception {
assertFalse(store.view(t1.getClass()).first(t2.id).skip(1).iterator().hasNext());
}

@Test
public void testDeleteParentIndex() throws Exception {
KVStore store = new InMemoryStore();

CustomType2 t1 = new CustomType2();
t1.key = "key1";
t1.id = "id1";
t1.parentId = "parentId1";
store.write(t1);

CustomType2 t2 = new CustomType2();
t2.key = "key2";
t2.id = "id2";
t2.parentId = "parentId1";
store.write(t2);

CustomType2 t3 = new CustomType2();
t3.key = "key3";
t3.id = "id1";
t3.parentId = "parentId2";
store.write(t3);

CustomType2 t4 = new CustomType2();
t4.key = "key4";
t4.id = "id2";
t4.parentId = "parentId2";
store.write(t4);

assertEquals(4, store.count(CustomType2.class));

store.delete(t1.getClass(), t1.key);
assertEquals(3, store.count(CustomType2.class));

store.delete(t2.getClass(), t2.key);
assertEquals(2, store.count(CustomType2.class));

store.delete(t3.getClass(), t3.key);
assertEquals(1, store.count(CustomType2.class));

store.delete(t4.getClass(), t4.key);
assertEquals(0, store.count(CustomType2.class));
}
}

0 comments on commit 0058212

Please sign in to comment.