BUG-8665: fix memory leak around RangeSets

This is a thinko on my part, where I was thinking in terms of a
discrete set (UnsignedLong) and assumed RangeSets will coalesce
individual items.

Unfortunately TreeRangeSet has no way of knowing that that the
domain it operates on is discrete and hence will not merge invididual
ranges.

This patch fixes the problem by using [N,N+1) ranges to address
the problem. A follow-up patch should address this in a more
efficient manner.

Change-Id: Iecc313e09ae0cdd51a42f7d39281f7634f0358a7
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
index 88c2a7d361a51ce384c87735e0918acec0a3019c..8563c3e913a87ea1de742b9a0b76b7ce3a1847e9 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
@@ -94,7 +94,7 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
                         closedTransactions = ImmutableMap.of();
                     }
 
                         closedTransactions = ImmutableMap.of();
                     }
 

-                    purgedTransactions.add(Range.singleton(ul));
+                    purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));

                     LOG.debug("{}: finished purging inherited transaction {}", persistenceId(), id);
                     envelope.sendSuccess(new TransactionPurgeResponse(id, request.getSequence()), readTime() - now);
                 });
                     LOG.debug("{}: finished purging inherited transaction {}", persistenceId(), id);
                     envelope.sendSuccess(new TransactionPurgeResponse(id, request.getSequence()), readTime() - now);
                 });


@@ -107,12 +107,12 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
                 // purged transactions in one go. If it does, we warn about the situation and
                 LOG.warn("{}: transaction {} not tracked in {}, but not present in active transactions", persistenceId,
                     id, purgedTransactions);
                 // purged transactions in one go. If it does, we warn about the situation and
                 LOG.warn("{}: transaction {} not tracked in {}, but not present in active transactions", persistenceId,
                     id, purgedTransactions);

-                purgedTransactions.add(Range.singleton(ul));
+                purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));

                 return new TransactionPurgeResponse(id, request.getSequence());
             }
 
             tree.purgeTransaction(id, () -> {
                 return new TransactionPurgeResponse(id, request.getSequence());
             }
 
             tree.purgeTransaction(id, () -> {

-                purgedTransactions.add(Range.singleton(ul));
+                purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));

                 transactions.remove(id);
                 LOG.debug("{}: finished purging transaction {}", persistenceId(), id);
                 envelope.sendSuccess(new TransactionPurgeResponse(id, request.getSequence()), readTime() - now);
                 transactions.remove(id);
                 LOG.debug("{}: finished purging transaction {}", persistenceId(), id);
                 envelope.sendSuccess(new TransactionPurgeResponse(id, request.getSequence()), readTime() - now);

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendClientMetadataBuilder.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendClientMetadataBuilder.java
index e4ab4b5ceff823663de9c6658cc1d98b16032537..519a360b06fd57245cfba8191d791939435ddfa1 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendClientMetadataBuilder.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendClientMetadataBuilder.java
@@ -110,7 +110,8 @@ final class FrontendClientMetadataBuilder implements Builder<FrontendClientMetad
         }
 
         // XXX: do we need to account for cookies?
         }
 
         // XXX: do we need to account for cookies?

-        purgedHistories.add(Range.singleton(UnsignedLong.fromLongBits(historyId.getHistoryId())));
+        final UnsignedLong ul = UnsignedLong.fromLongBits(historyId.getHistoryId());
+        purgedHistories.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));

         LOG.debug("{}: Purged history {}", historyId);
     }
 
         LOG.debug("{}: Purged history {}", historyId);
     }
 

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendHistoryMetadataBuilder.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendHistoryMetadataBuilder.java
index beed765a690590bd8174210861a06c7a2b78f09c..e787edb7a80e391bc2d8bd900535f2b2836b8d10 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendHistoryMetadataBuilder.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendHistoryMetadataBuilder.java
@@ -71,7 +71,7 @@ final class FrontendHistoryMetadataBuilder implements Builder<FrontendHistoryMet
     void onTransactionPurged(final TransactionIdentifier txId) {
         final UnsignedLong id = UnsignedLong.fromLongBits(txId.getTransactionId());
         closedTransactions.remove(id);
     void onTransactionPurged(final TransactionIdentifier txId) {
         final UnsignedLong id = UnsignedLong.fromLongBits(txId.getTransactionId());
         closedTransactions.remove(id);

-        purgedTransactions.add(Range.singleton(id));
+        purgedTransactions.add(Range.closedOpen(id, UnsignedLong.ONE.plus(id)));

     }
 
     /**
     }
 
     /**

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java
index 1ea3c53a5fdb7a5f7a2edcf972948ca5fdb8ef2d..d860bfa289e39f7b9a144aa652a5a030ec4c4546 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java
@@ -179,7 +179,8 @@ final class LeaderFrontendState implements Identifiable<ClientIdentifier> {
         }
 
         LOG.debug("{}: purging history {}", persistenceId, id);
         }
 
         LOG.debug("{}: purging history {}", persistenceId, id);

-        purgedHistories.add(Range.singleton(UnsignedLong.fromLongBits(id.getHistoryId())));
+        final UnsignedLong ul = UnsignedLong.fromLongBits(id.getHistoryId());
+        purgedHistories.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));

         existing.purge(request.getSequence(), envelope, now);
         return null;
     }
         existing.purge(request.getSequence(), envelope, now);
         return null;
     }