You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@giraph.apache.org by ap...@apache.org on 2013/06/01 00:35:09 UTC
git commit: updated refs/heads/trunk to c5fe858
Updated Branches:
refs/heads/trunk 458ddd8f9 -> c5fe858e9
GIRAPH-675: Mutable edge iterator gets corrupted by calling vertex.getNumEdges() during iteration (apresta)
Project: http://git-wip-us.apache.org/repos/asf/giraph/repo
Commit: http://git-wip-us.apache.org/repos/asf/giraph/commit/c5fe858e
Tree: http://git-wip-us.apache.org/repos/asf/giraph/tree/c5fe858e
Diff: http://git-wip-us.apache.org/repos/asf/giraph/diff/c5fe858e
Branch: refs/heads/trunk
Commit: c5fe858e9cf7ccd5746aa1168414b42b79c4adc2
Parents: 458ddd8
Author: Alessandro Presta <al...@fb.com>
Authored: Fri May 31 14:57:11 2013 -0700
Committer: Alessandro Presta <al...@fb.com>
Committed: Fri May 31 15:34:47 2013 -0700
----------------------------------------------------------------------
CHANGELOG | 3 ++
.../apache/giraph/edge/MutableEdgesIterable.java | 1 +
.../apache/giraph/edge/MutableEdgesWrapper.java | 13 ++++++++-
.../main/java/org/apache/giraph/graph/Vertex.java | 5 +++-
.../apache/giraph/graph/TestVertexAndEdges.java | 21 ++++++++++++++-
5 files changed, 40 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/giraph/blob/c5fe858e/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 66fdd69..99d6958 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,9 @@
Giraph Change Log
Release 1.1.0 - unreleased
+ GIRAPH-675: Mutable edge iterator gets corrupted by calling
+ vertex.getNumEdges() during iteration (apresta)
+
GIRAPH-512: JavaDoc warnings (emre.aladag via nitay)
GIRAPH-620: Website Documentation: How to use Hive I/O with Giraph (nitay)
http://git-wip-us.apache.org/repos/asf/giraph/blob/c5fe858e/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesIterable.java
----------------------------------------------------------------------
diff --git a/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesIterable.java b/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesIterable.java
index ca32a9f..ea55e29 100644
--- a/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesIterable.java
+++ b/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesIterable.java
@@ -96,6 +96,7 @@ public class MutableEdgesIterable<I extends WritableComparable,
// Set the current edge to null, so that it's not added to the
// new edges.
mutableEdgesWrapper.setCurrentEdge(null);
+ mutableEdgesWrapper.decrementEdges();
}
};
}
http://git-wip-us.apache.org/repos/asf/giraph/blob/c5fe858e/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesWrapper.java
----------------------------------------------------------------------
diff --git a/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesWrapper.java b/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesWrapper.java
index 529234d..4e2a2a2 100644
--- a/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesWrapper.java
+++ b/giraph-core/src/main/java/org/apache/giraph/edge/MutableEdgesWrapper.java
@@ -47,6 +47,8 @@ public class MutableEdgesWrapper<I extends WritableComparable,
private final Iterator<Edge<I, E>> oldEdgesIterator;
/** Last edge that was returned during iteration. */
private MutableEdge<I, E> currentEdge;
+ /** Number of edges. */
+ private int numEdges;
/**
* Private constructor: instantiation happens through the {@code wrap()}
@@ -59,6 +61,7 @@ public class MutableEdgesWrapper<I extends WritableComparable,
OutEdges<I, E> newEdges) {
oldEdgesIterator = oldEdges.iterator();
this.newEdges = newEdges;
+ numEdges = oldEdges.size();
}
/**
@@ -131,6 +134,14 @@ public class MutableEdgesWrapper<I extends WritableComparable,
currentEdge = edge;
}
+ /**
+ * Decrement the number of edges (to account for a deletion from the mutable
+ * iterator).
+ */
+ public void decrementEdges() {
+ --numEdges;
+ }
+
@Override
public void initialize(Iterable<Edge<I, E>> edges) {
throw new IllegalStateException("initialize: MutableEdgesWrapper should " +
@@ -161,7 +172,7 @@ public class MutableEdgesWrapper<I extends WritableComparable,
@Override
public int size() {
- return unwrap().size();
+ return numEdges;
}
@Override
http://git-wip-us.apache.org/repos/asf/giraph/blob/c5fe858e/giraph-core/src/main/java/org/apache/giraph/graph/Vertex.java
----------------------------------------------------------------------
diff --git a/giraph-core/src/main/java/org/apache/giraph/graph/Vertex.java b/giraph-core/src/main/java/org/apache/giraph/graph/Vertex.java
index 82fbe0c..1241ae7 100644
--- a/giraph-core/src/main/java/org/apache/giraph/graph/Vertex.java
+++ b/giraph-core/src/main/java/org/apache/giraph/graph/Vertex.java
@@ -129,6 +129,8 @@ public class Vertex<I extends WritableComparable,
* Note: edge objects returned by this iterable may be invalidated as soon
* as the next element is requested. Thus, keeping a reference to an edge
* almost always leads to undesired behavior.
+ * Accessing the edges with other methods (e.g., addEdge()) during iteration
+ * leads to undefined behavior.
*
* @return the out edges (sort order determined by subclass implementation).
*/
@@ -140,7 +142,8 @@ public class Vertex<I extends WritableComparable,
* Get an iterable of out-edges that can be modified in-place.
* This can mean changing the current edge value or removing the current edge
* (by using the iterator version).
- * Note: if
+ * Note: accessing the edges with other methods (e.g., addEdge()) during
+ * iteration leads to undefined behavior.
*
* @return An iterable of mutable out-edges
*/
http://git-wip-us.apache.org/repos/asf/giraph/blob/c5fe858e/giraph-core/src/test/java/org/apache/giraph/graph/TestVertexAndEdges.java
----------------------------------------------------------------------
diff --git a/giraph-core/src/test/java/org/apache/giraph/graph/TestVertexAndEdges.java b/giraph-core/src/test/java/org/apache/giraph/graph/TestVertexAndEdges.java
index b6e17fd..d0a6c46 100644
--- a/giraph-core/src/test/java/org/apache/giraph/graph/TestVertexAndEdges.java
+++ b/giraph-core/src/test/java/org/apache/giraph/graph/TestVertexAndEdges.java
@@ -308,7 +308,6 @@ public class TestVertexAndEdges {
int i = 2;
for (MutableEdge<LongWritable, DoubleWritable> edge :
vertex.getMutableEdges()) {
- System.out.println(edge.toString());
if (i-- == 0) {
break;
}
@@ -323,6 +322,26 @@ public class TestVertexAndEdges {
}
}
assertEquals(5, vertex.getNumEdges());
+
+ // Calling size() during iteration shouldn't modify the data structure.
+ int iterations = 0;
+ for (MutableEdge<LongWritable, DoubleWritable> edge : vertex.getMutableEdges()) {
+ edge.setValue(new DoubleWritable(3));
+ assertEquals(5, vertex.getNumEdges());
+ ++iterations;
+ }
+ assertEquals(5, vertex.getNumEdges());
+ assertEquals(5, iterations);
+
+ // If we remove an edge after calling next(), size() should return the
+ // correct number of edges.
+ it = vertex.getMutableEdges().iterator();
+ it.next();
+ it.remove();
+ assertEquals(4, vertex.getNumEdges());
+ it.next();
+ it.remove();
+ assertEquals(3, vertex.getNumEdges());
}
/**