-
Notifications
You must be signed in to change notification settings - Fork 521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(api&core): in oltp apis, add statistics info and support full info about vertices and edges #2262
Conversation
ApiMeasure will count the number of vertices and edges traversed at runtime, and the time the api takes to execute
…rface * JsonSerializer: return measure information in api response * Serializer: fit the feature that returns complete information about vertices and edges
…d Support full information about vertices and edges Statistics information: * add vertexIterCounter and edgeIterCounter in HugeTraverser.java to track traversed vertices and edges at run time * modify all oltp restful apis to add statistics information in response Full information about vertices and edges: * add 'with_vertex' and 'with_edge' parameter option in apis * modify oltp apis to support vertex and edge information in api response * add EdgeRecord in HugeTraverser.java to record edges at run time and generate the edge information returned in api response * modify Path and PathSet in HugeTraverser.java to support full edge information storage * modify all traversers to support track of edge information at run time
...graph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/SameNeighborTraverser.java
Fixed
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #2262 +/- ##
============================================
- Coverage 68.63% 65.08% -3.56%
- Complexity 977 981 +4
============================================
Files 498 498
Lines 40684 41240 +556
Branches 5681 5738 +57
============================================
- Hits 27922 26839 -1083
- Misses 10056 11746 +1690
+ Partials 2706 2655 -51
... and 72 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/KneighborAPI.java
Outdated
Show resolved
Hide resolved
...ph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/CollectionPathsTraverser.java
Show resolved
Hide resolved
...e/src/main/java/org/apache/hugegraph/traversal/algorithm/MultiNodeShortestPathTraverser.java
Show resolved
Hide resolved
return manager.serializer(g).writePaths("crosspoints", paths, true); | ||
measure.addIterCount(traverser.vertexIterCounter.get(), | ||
traverser.edgeIterCounter.get()); | ||
return manager.serializer(g, measure.getResult()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer to update the serializer(g)
method and set default an ApiMeasure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rejected:
ApiMeasurer
should be passed in from an outside api implementation so that it can carry data- not all apis need
ApiMeasurer
and keeping theApiMeasurer
null for those APIs can preventApiMeasurer
from being serialized
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/CustomizedCrosspointsAPI.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/CustomizedCrosspointsAPI.java
Outdated
Show resolved
Hide resolved
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/AllShortestPathsAPI.java
Outdated
Show resolved
Hide resolved
d468e18
to
e671963
Compare
} | ||
|
||
public Map<String, Object> measures() { | ||
measures.put(COST, System.currentTimeMillis() - timeStart); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compute duration use System.nanoTime perfered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
} else if (current instanceof MutableLong) { | ||
MutableLong currentMutableLong = (MutableLong) current; | ||
currentMutableLong.add(value); | ||
} else if (current instanceof Long) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
measures.computeIfAbsent(key, MutableLong.new).add(value)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
private static CollectionFactory collectionFactory; | ||
private final HugeGraph graph; | ||
// for apimeasure | ||
public AtomicLong edgeIterCounter = new AtomicLong(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better for this class to have no side effects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HugeTraverser is the root class of all traverser algorithms, and there is a need for all sub-traverser algorithms to count the measuring statistics during traversering. So I think of two ways to solve this problems:
- Give HugeTraverser the ability to track the measuring statistics;
- Track the measuing statistics in all traverser functions which the Traverser APIs would call
I think the first one is better & more elegant and this is the current implementation.
.gitignore
Outdated
swagger-ui-* | ||
hugegraph-dist/dist.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, but we need fix/remove it in the dist-script
rather than exclude them in git (will be fixed it in another PR) @VGalaxies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, fixed and rollback change
public static final String JSON = MediaType.APPLICATION_JSON_TYPE | ||
.getSubtype(); | ||
|
||
.getSubtype(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we keep the origin style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -224,8 +220,7 @@ protected void addCount(String key, long value) { | |||
if (current == null) { | |||
measures.put(key, new MutableLong(value)); | |||
} else if (current instanceof MutableLong) { | |||
MutableLong currentMutableLong = (MutableLong) current; | |||
currentMutableLong.add(value); | |||
((MutableLong) measures.computeIfAbsent(key, MutableLong::new)).add(value); | |||
} else if (current instanceof Long) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add else branch then throw an exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@Path("graphs/{graph}/traversers/customizedpaths") | ||
@Singleton | ||
@Tag(name = "CustomizedPathsAPI") | ||
public class CustomizedPathsAPI extends API { | ||
|
||
private static final Logger LOG = Log.logger(CustomizedPathsAPI.class); | ||
|
||
private static List<WeightedEdgeStep> step(HugeGraph graph, | ||
PathRequest req) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also keep request name style?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
HugeGraph g = graph(manager, graph); | ||
Iterator<Vertex> sources = request.sources.vertices(g); | ||
E.checkArgument(sources != null && sources.hasNext(), | ||
"The source vertices can't be empty"); | ||
|
||
FusiformSimilarityTraverser traverser = | ||
new FusiformSimilarityTraverser(g); | ||
new FusiformSimilarityTraverser(g); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems one line is ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
E.checkArgument(!request.withVertex && !request.withPath, | ||
"Can't return vertex or path when count only"); | ||
E.checkArgument(!request.withVertex && !request.withPath && !request.withEdge, | ||
"Can't return vertex or path or edge when count only"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't return vertex, edge or path...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
public class CustomizedCrosspointsTraverser extends HugeTraverser { | ||
|
||
private final EdgeRecord edgeRecord; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
public class CustomizePathsTraverser extends HugeTraverser { | ||
private final EdgeRecord edgeRecord; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expect a blank line, and prefer to rename to edgeResults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -52,10 +50,11 @@ public abstract class PathTraverser { | |||
protected Set<HugeTraverser.Path> paths; | |||
|
|||
protected TraverseStrategy traverseStrategy; | |||
protected HugeTraverser.EdgeRecord edgeRecord; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just import EdgeRecord to keep the same style?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
return paths; | ||
} | ||
|
||
private class Traverser { | ||
|
||
private final ShortestPathRecords record; | ||
private final EdgeRecord edgeRecord; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to pathResults and edgeResults
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
result.add(nodes.get(random)); | ||
} | ||
return result; | ||
public EdgeRecord getEdgeRecord() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
* rename edgeRecord to edgeResults * fix Request class code style in SameNeighborsAPI.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Good job, thanks for ur contributuion & reaction 'll check this pr carefully later, @zyxxoo could ensure it now? |
for (Path path : paths) { | ||
List<Id> vertices = path.vertices(); | ||
int length = vertices.size(); | ||
if (intersection.contains(vertices.get(length - 1))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try keep origin code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean to leave this static method in its original place?
I just formatted it automatically. The content of this method is the same as the original, and only the location has changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will check it later(merge it first)
…fo about vertices and edges (apache#2262) * chore: improve gitignore file * feat: add ApiMeasure to collect runtime data ApiMeasure will count the number of vertices and edges traversed at runtime, and the time the api takes to execute * feat: Add ApiMeasure to JsonSerializer and Modify the Serializer interface * JsonSerializer: return measure information in api response * Serializer: fit the feature that returns complete information about vertices and edges * refactor: format code based on hugegraph-style.xml * feat: Add statistics information in all oltp restful apis response and Support full information about vertices and edges Statistics information: * add vertexIterCounter and edgeIterCounter in HugeTraverser.java to track traversed vertices and edges at run time * modify all oltp restful apis to add statistics information in response Full information about vertices and edges: * add 'with_vertex' and 'with_edge' parameter option in apis * modify oltp apis to support vertex and edge information in api response * add EdgeRecord in HugeTraverser.java to record edges at run time and generate the edge information returned in api response * modify Path and PathSet in HugeTraverser.java to support full edge information storage * modify all traversers to support track of edge information at run time * fix: numeric cast * fix: Jaccard Similarity api test * fix: adjust the code style and naming convention * Empty commit * Empty commit * fix: 1. change System.currentTimeMillis() to System.nanoTime(); 2. modify addCount() * fix: rollback change in .gitignore * fix: rollback ServerOptions.java code style * fix: rollback API.java code style and add exception in else branch * fix: fix code style * fix: name style & code style * rename edgeRecord to edgeResults * fix Request class code style in SameNeighborsAPI.java
…fo about vertices and edges (apache#2262) * chore: improve gitignore file * feat: add ApiMeasure to collect runtime data ApiMeasure will count the number of vertices and edges traversed at runtime, and the time the api takes to execute * feat: Add ApiMeasure to JsonSerializer and Modify the Serializer interface * JsonSerializer: return measure information in api response * Serializer: fit the feature that returns complete information about vertices and edges * refactor: format code based on hugegraph-style.xml * feat: Add statistics information in all oltp restful apis response and Support full information about vertices and edges Statistics information: * add vertexIterCounter and edgeIterCounter in HugeTraverser.java to track traversed vertices and edges at run time * modify all oltp restful apis to add statistics information in response Full information about vertices and edges: * add 'with_vertex' and 'with_edge' parameter option in apis * modify oltp apis to support vertex and edge information in api response * add EdgeRecord in HugeTraverser.java to record edges at run time and generate the edge information returned in api response * modify Path and PathSet in HugeTraverser.java to support full edge information storage * modify all traversers to support track of edge information at run time * fix: numeric cast * fix: Jaccard Similarity api test * fix: adjust the code style and naming convention * Empty commit * Empty commit * fix: 1. change System.currentTimeMillis() to System.nanoTime(); 2. modify addCount() * fix: rollback change in .gitignore * fix: rollback ServerOptions.java code style * fix: rollback API.java code style and add exception in else branch * fix: fix code style * fix: name style & code style * rename edgeRecord to edgeResults * fix Request class code style in SameNeighborsAPI.java
…fo about vertices and edges (#2262) * chore: improve gitignore file * feat: add ApiMeasure to collect runtime data ApiMeasure will count the number of vertices and edges traversed at runtime, and the time the api takes to execute * feat: Add ApiMeasure to JsonSerializer and Modify the Serializer interface * JsonSerializer: return measure information in api response * Serializer: fit the feature that returns complete information about vertices and edges * refactor: format code based on hugegraph-style.xml * feat: Add statistics information in all oltp restful apis response and Support full information about vertices and edges Statistics information: * add vertexIterCounter and edgeIterCounter in HugeTraverser.java to track traversed vertices and edges at run time * modify all oltp restful apis to add statistics information in response Full information about vertices and edges: * add 'with_vertex' and 'with_edge' parameter option in apis * modify oltp apis to support vertex and edge information in api response * add EdgeRecord in HugeTraverser.java to record edges at run time and generate the edge information returned in api response * modify Path and PathSet in HugeTraverser.java to support full edge information storage * modify all traversers to support track of edge information at run time * fix: numeric cast * fix: Jaccard Similarity api test * fix: adjust the code style and naming convention * Empty commit * Empty commit * fix: 1. change System.currentTimeMillis() to System.nanoTime(); 2. modify addCount() * fix: rollback change in .gitignore * fix: rollback ServerOptions.java code style * fix: rollback API.java code style and add exception in else branch * fix: fix code style * fix: name style & code style * rename edgeRecord to edgeResults * fix Request class code style in SameNeighborsAPI.java
Purpose of the PR
Main Changes
Statistics information:
add vertexIterCounter and edgeIterCounter in HugeTraverser.java to track traversed vertices and edges at run time
modify all oltp apis to add statistics information in response
modify Serializer.java and JsonSerializer.java to support statistics infomation serialization
design document
Full information about vertices and edges:
add 'with_vertex' and 'with_edge' parameter option in oltp apis
modify oltp apis to support vertex and edge information in api response
add EdgeRecord in HugeTraverser.java to record edges at run time and generate the edge information returned in api response
modify Path and PathSet in HugeTraverser.java to support full edge information storage
modify all traversers to support track of edge information at run time
modify Serializer.java and JsonSerializer.java to support full infomation serialization about vertices and edges
design document
Implementation overview
N: No Need
✓: Supported
Verifying these changes
Kout Post
andSingle Source Shortest Path Get
are shown belowInitialize the Graph
Kout Post
with_vertex=false, with_edge=false
with_vertex=true, with_edge=true
Single Source Shortest Path Get
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODO
Doc - Done
Doc - No Need