Skip to content

Commit

Permalink
Fix NPE when moving unregistered query between memory pools
Browse files Browse the repository at this point in the history
  • Loading branch information
dain committed May 7, 2019
1 parent 7e75aac commit bd75aa4
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
5 changes: 5 additions & 0 deletions presto-main/src/main/java/io/prestosql/memory/MemoryPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.google.common.util.concurrent.AbstractFuture;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import io.airlift.units.DataSize;
import io.prestosql.spi.QueryId;
Expand Down Expand Up @@ -250,6 +251,10 @@ synchronized ListenableFuture<?> moveQuery(QueryId queryId, MemoryPool targetMem
long originalRevocableReserved = getQueryRevocableMemoryReservation(queryId);
// Get the tags before we call free() as that would remove the tags and we will lose the tags.
Map<String, Long> taggedAllocations = taggedMemoryAllocations.remove(queryId);
if (taggedAllocations == null) {
// query is not registered (likely a race with query completion)
return Futures.immediateFuture(null);
}
ListenableFuture<?> future = targetMemoryPool.reserve(queryId, MOVE_QUERY_TAG, originalReserved);
free(queryId, MOVE_QUERY_TAG, originalReserved);
targetMemoryPool.reserveRevocable(queryId, originalRevocableReserved);
Expand Down
14 changes: 14 additions & 0 deletions presto-main/src/test/java/io/prestosql/memory/TestMemoryPools.java
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,20 @@ public void testMoveQuery()
assertEquals(pool2.getFreeBytes(), 1000);
}

@Test
public void testMoveUnknownQuery()
{
QueryId testQuery = new QueryId("test_query");
MemoryPool pool1 = new MemoryPool(new MemoryPoolId("test"), new DataSize(1000, BYTE));
MemoryPool pool2 = new MemoryPool(new MemoryPoolId("test"), new DataSize(1000, BYTE));

assertNull(pool1.getTaggedMemoryAllocations().get(testQuery));

pool1.moveQuery(testQuery, pool2);
assertNull(pool1.getTaggedMemoryAllocations().get(testQuery));
assertNull(pool2.getTaggedMemoryAllocations().get(testQuery));
}

private long runDriversUntilBlocked(Predicate<OperatorContext> reason)
{
long iterationsCount = 0;
Expand Down

0 comments on commit bd75aa4

Please sign in to comment.