This is an automated email from the ASF dual-hosted git repository.
gengliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 78721fd [SPARK-31014][CORE] InMemoryStore: remove key from
parentToChildrenMap when removing key from CountingRemoveIfForEach
78721fd is described below
commit 78721fd8c5cc2b09807c7a3196244cac60bcfbeb
Author: Jungtaek Lim (HeartSaVioR) <[email protected]>
AuthorDate: Wed Mar 4 10:05:26 2020 -0800
[SPARK-31014][CORE] InMemoryStore: remove key from parentToChildrenMap when
removing key from CountingRemoveIfForEach
### What changes were proposed in this pull request?
This patch addresses missed spot on SPARK-30964 (#27716) - SPARK-30964
added secondary index which defines the relationship between parent - children
and able to operate all children for given parent faster.
While SPARK-30964 handled the addition and deletion of secondary index in
InstanceList properly, it missed to add code to handle deletion of secondary
index in CountingRemoveIfForEach, resulting to the leak of indices. This patch
adds the deletion of secondary index in CountingRemoveIfForEach.
### Why are the changes needed?
Described above.
### Does this PR introduce any user-facing change?
No.
### How was this patch tested?
N/A, as relevant field and class are marked as private, and it cannot be
checked in higher level. I'm not sure we want to adjust scope to add a test.
Closes #27765 from HeartSaVioR/SPARK-31014.
Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Gengliang Wang <[email protected]>
---
.../apache/spark/util/kvstore/InMemoryStore.java | 30 +++++++++++++++-------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git
a/common/kvstore/src/main/java/org/apache/spark/util/kvstore/InMemoryStore.java
b/common/kvstore/src/main/java/org/apache/spark/util/kvstore/InMemoryStore.java
index f1bebb4..e929c6c 100644
---
a/common/kvstore/src/main/java/org/apache/spark/util/kvstore/InMemoryStore.java
+++
b/common/kvstore/src/main/java/org/apache/spark/util/kvstore/InMemoryStore.java
@@ -177,7 +177,7 @@ public class InMemoryStore implements KVStore {
* iterators. https://bugs.openjdk.java.net/browse/JDK-8078645
*/
private static class CountingRemoveIfForEach<T> implements
BiConsumer<Comparable<Object>, T> {
- private final ConcurrentMap<Comparable<Object>, T> data;
+ private final InstanceList<T> instanceList;
private final Predicate<? super T> filter;
/**
@@ -189,17 +189,15 @@ public class InMemoryStore implements KVStore {
*/
private int count = 0;
- CountingRemoveIfForEach(
- ConcurrentMap<Comparable<Object>, T> data,
- Predicate<? super T> filter) {
- this.data = data;
+ CountingRemoveIfForEach(InstanceList<T> instanceList, Predicate<? super
T> filter) {
+ this.instanceList = instanceList;
this.filter = filter;
}
@Override
public void accept(Comparable<Object> key, T value) {
if (filter.test(value)) {
- if (data.remove(key, value)) {
+ if (instanceList.delete(key, value)) {
count++;
}
}
@@ -253,7 +251,7 @@ public class InMemoryStore implements KVStore {
return count;
} else {
Predicate<? super T> filter = getPredicate(ti.getAccessor(index),
indexValues);
- CountingRemoveIfForEach<T> callback = new
CountingRemoveIfForEach<>(data, filter);
+ CountingRemoveIfForEach<T> callback = new
CountingRemoveIfForEach<>(this, filter);
// Go through all the values in `data` and delete objects that meets
the predicate `filter`.
// This can be slow when there is a large number of entries in `data`.
@@ -278,7 +276,22 @@ public class InMemoryStore implements KVStore {
public boolean delete(Object key) {
boolean entryExists = data.remove(asKey(key)) != null;
- if (entryExists && hasNaturalParentIndex) {
+ if (entryExists) {
+ deleteParentIndex(key);
+ }
+ return entryExists;
+ }
+
+ public boolean delete(Object key, T value) {
+ boolean entryExists = data.remove(asKey(key), value);
+ if (entryExists) {
+ deleteParentIndex(key);
+ }
+ return entryExists;
+ }
+
+ private void deleteParentIndex(Object key) {
+ if (hasNaturalParentIndex) {
for (NaturalKeys v : parentToChildrenMap.values()) {
if (v.remove(asKey(key))) {
// `v` can be empty after removing the natural key and we can
remove it from
@@ -289,7 +302,6 @@ public class InMemoryStore implements KVStore {
}
}
}
- return entryExists;
}
public int size() {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
