You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Nitay Joffe <ni...@apache.org> on 2012/11/09 01:07:40 UTC
Review Request: GIRAPH-415: Refactor / cleanup Hadoop Counters
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7980/
-----------------------------------------------------------
Review request for giraph.
Description
-------
GIRAPH-415: Refactor / cleanup Hadoop Counters
Diffs
-----
giraph/src/main/java/org/apache/giraph/counters/GiraphHadoopCounter.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/counters/GiraphTimers.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/counters/HadoopCountersBase.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/counters/package-info.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
giraph/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 8801dde5102ffca397678b0dcc3dd7c2932d22d7
giraph/src/main/java/org/apache/giraph/graph/HashMapVertex.java 26b14caed23706b12f21142862bbd15fe3720bd6
giraph/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java a23b87f7e43d6a3b78516cbb5f1dd512dca84a8d
giraph/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1db6e977eb4a8102351a908dc1a113935002e61e
giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
giraph/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java 85af9700e76a3ef32191fe0dc5631f430b13de42
giraph/src/main/java/org/apache/giraph/graph/SimpleVertex.java ed6759a884553fa15ee477e19e483e1d210ba7be
giraph/src/main/java/org/apache/giraph/graph/Vertex.java 8789ee5c8f170b6d598f0b36053333b1f6f36227
giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java 262db29bbd4797e470f426999b85f2c1a7d1dee3
giraph/src/main/java/org/apache/giraph/metrics/GiraphMetricsRegistry.java eac9f7295d968c3b30c40b89944e8f9f8549cc70
giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/package-info.java f7529f52527e8e4253f579bd6d01865aa980926b
Diff: https://reviews.apache.org/r/7980/diff/
Testing
-------
Thanks,
Nitay Joffe
Re: Review Request: GIRAPH-415: Refactor / cleanup Hadoop Counters
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7980/#review13368
-----------------------------------------------------------
Ship it!
+1, thanks Nitay.
- Maja Kabiljo
On Nov. 12, 2012, 6:59 p.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7980/
> -----------------------------------------------------------
>
> (Updated Nov. 12, 2012, 6:59 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-415: Refactor / cleanup Hadoop Counters
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/counters/GiraphHadoopCounter.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/GiraphTimers.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/HadoopCountersBase.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/package-info.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb
> giraph/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 8801dde
> giraph/src/main/java/org/apache/giraph/graph/HashMapVertex.java 26b14ca
> giraph/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java a23b87f
> giraph/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1db6e97
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d
> giraph/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java 85af970
> giraph/src/main/java/org/apache/giraph/graph/SimpleVertex.java ed6759a
> giraph/src/main/java/org/apache/giraph/graph/Vertex.java 8789ee5
> giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java 262db29
> giraph/src/main/java/org/apache/giraph/metrics/GiraphMetricsRegistry.java eac9f72
> giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/package-info.java f7529f5
>
> Diff: https://reviews.apache.org/r/7980/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-415: Refactor / cleanup Hadoop Counters
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7980/
-----------------------------------------------------------
(Updated Nov. 12, 2012, 6:59 p.m.)
Review request for giraph.
Changes
-------
Maja's comments
Description
-------
GIRAPH-415: Refactor / cleanup Hadoop Counters
Diffs (updated)
-----
giraph/src/main/java/org/apache/giraph/counters/GiraphHadoopCounter.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/counters/GiraphTimers.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/counters/HadoopCountersBase.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/counters/package-info.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb
giraph/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 8801dde
giraph/src/main/java/org/apache/giraph/graph/HashMapVertex.java 26b14ca
giraph/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java a23b87f
giraph/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1db6e97
giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d
giraph/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java 85af970
giraph/src/main/java/org/apache/giraph/graph/SimpleVertex.java ed6759a
giraph/src/main/java/org/apache/giraph/graph/Vertex.java 8789ee5
giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java 262db29
giraph/src/main/java/org/apache/giraph/metrics/GiraphMetricsRegistry.java eac9f72
giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java PRE-CREATION
giraph/src/main/java/org/apache/giraph/metrics/package-info.java f7529f5
Diff: https://reviews.apache.org/r/7980/diff/
Testing
-------
Thanks,
Nitay Joffe
Re: Review Request: GIRAPH-415: Refactor / cleanup Hadoop Counters
Posted by Nitay Joffe <ni...@apache.org>.
> On Nov. 9, 2012, 7:03 p.m., Maja Kabiljo wrote:
> > giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java, line 170
> > <https://reviews.apache.org/r/7980/diff/1/?file=187458#file187458line170>
> >
> > Am I missing something or this is not used?
> > I understand the idea of it being Iterable, but why don't make HadoopCountersBase Iterable then?
It's to make it easier for users. Done, made HadoopCounterBase Iterable.
> On Nov. 9, 2012, 7:03 p.m., Maja Kabiljo wrote:
> > giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java, line 769
> > <https://reviews.apache.org/r/7980/diff/1/?file=187462#file187462line769>
> >
> > You can use setValue here
That makes the logic different than before though? I mean it was using increment() before too, so this have a different effect?
- Nitay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7980/#review13296
-----------------------------------------------------------
On Nov. 9, 2012, 12:07 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7980/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 12:07 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-415: Refactor / cleanup Hadoop Counters
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/counters/GiraphHadoopCounter.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/GiraphTimers.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/HadoopCountersBase.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/package-info.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
> giraph/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 8801dde5102ffca397678b0dcc3dd7c2932d22d7
> giraph/src/main/java/org/apache/giraph/graph/HashMapVertex.java 26b14caed23706b12f21142862bbd15fe3720bd6
> giraph/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java a23b87f7e43d6a3b78516cbb5f1dd512dca84a8d
> giraph/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1db6e977eb4a8102351a908dc1a113935002e61e
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
> giraph/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java 85af9700e76a3ef32191fe0dc5631f430b13de42
> giraph/src/main/java/org/apache/giraph/graph/SimpleVertex.java ed6759a884553fa15ee477e19e483e1d210ba7be
> giraph/src/main/java/org/apache/giraph/graph/Vertex.java 8789ee5c8f170b6d598f0b36053333b1f6f36227
> giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java 262db29bbd4797e470f426999b85f2c1a7d1dee3
> giraph/src/main/java/org/apache/giraph/metrics/GiraphMetricsRegistry.java eac9f7295d968c3b30c40b89944e8f9f8549cc70
> giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/package-info.java f7529f52527e8e4253f579bd6d01865aa980926b
>
> Diff: https://reviews.apache.org/r/7980/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-415: Refactor / cleanup Hadoop Counters
Posted by Maja Kabiljo <ma...@fb.com>.
> On Nov. 9, 2012, 7:03 p.m., Maja Kabiljo wrote:
> > giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java, line 769
> > <https://reviews.apache.org/r/7980/diff/1/?file=187462#file187462line769>
> >
> > You can use setValue here
>
> Nitay Joffe wrote:
> That makes the logic different than before though? I mean it was using increment() before too, so this have a different effect?
The way it was before isn't correct, right? It just worked when we restart the application because the counter value is zero...
- Maja
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7980/#review13296
-----------------------------------------------------------
On Nov. 9, 2012, 12:07 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7980/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 12:07 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-415: Refactor / cleanup Hadoop Counters
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/counters/GiraphHadoopCounter.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/GiraphTimers.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/HadoopCountersBase.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/package-info.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
> giraph/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 8801dde5102ffca397678b0dcc3dd7c2932d22d7
> giraph/src/main/java/org/apache/giraph/graph/HashMapVertex.java 26b14caed23706b12f21142862bbd15fe3720bd6
> giraph/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java a23b87f7e43d6a3b78516cbb5f1dd512dca84a8d
> giraph/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1db6e977eb4a8102351a908dc1a113935002e61e
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
> giraph/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java 85af9700e76a3ef32191fe0dc5631f430b13de42
> giraph/src/main/java/org/apache/giraph/graph/SimpleVertex.java ed6759a884553fa15ee477e19e483e1d210ba7be
> giraph/src/main/java/org/apache/giraph/graph/Vertex.java 8789ee5c8f170b6d598f0b36053333b1f6f36227
> giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java 262db29bbd4797e470f426999b85f2c1a7d1dee3
> giraph/src/main/java/org/apache/giraph/metrics/GiraphMetricsRegistry.java eac9f7295d968c3b30c40b89944e8f9f8549cc70
> giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/package-info.java f7529f52527e8e4253f579bd6d01865aa980926b
>
> Diff: https://reviews.apache.org/r/7980/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>
Re: Review Request: GIRAPH-415: Refactor / cleanup Hadoop Counters
Posted by Maja Kabiljo <ma...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7980/#review13296
-----------------------------------------------------------
Looks good.
giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java
<https://reviews.apache.org/r/7980/#comment28561>
Am I missing something or this is not used?
I understand the idea of it being Iterable, but why don't make HadoopCountersBase Iterable then?
giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java
<https://reviews.apache.org/r/7980/#comment28560>
You can use setValue here
giraph/src/main/java/org/apache/giraph/graph/Vertex.java
<https://reviews.apache.org/r/7980/#comment28559>
Nice refactoring.
- Maja Kabiljo
On Nov. 9, 2012, 12:07 a.m., Nitay Joffe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7980/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2012, 12:07 a.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> GIRAPH-415: Refactor / cleanup Hadoop Counters
>
>
> Diffs
> -----
>
> giraph/src/main/java/org/apache/giraph/counters/GiraphHadoopCounter.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/GiraphStats.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/GiraphTimers.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/HadoopCountersBase.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/counters/package-info.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/graph/BspServiceMaster.java 7d5dabb9df619b82dd42ab631b3c244fbdd9ddcf
> giraph/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 8801dde5102ffca397678b0dcc3dd7c2932d22d7
> giraph/src/main/java/org/apache/giraph/graph/HashMapVertex.java 26b14caed23706b12f21142862bbd15fe3720bd6
> giraph/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java a23b87f7e43d6a3b78516cbb5f1dd512dca84a8d
> giraph/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1db6e977eb4a8102351a908dc1a113935002e61e
> giraph/src/main/java/org/apache/giraph/graph/MasterThread.java 5c9a72d5edbaafddca1f5050131a1657e15f5ac2
> giraph/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java 85af9700e76a3ef32191fe0dc5631f430b13de42
> giraph/src/main/java/org/apache/giraph/graph/SimpleVertex.java ed6759a884553fa15ee477e19e483e1d210ba7be
> giraph/src/main/java/org/apache/giraph/graph/Vertex.java 8789ee5c8f170b6d598f0b36053333b1f6f36227
> giraph/src/main/java/org/apache/giraph/metrics/EmptyMetricsRegistry.java 262db29bbd4797e470f426999b85f2c1a7d1dee3
> giraph/src/main/java/org/apache/giraph/metrics/GiraphMetricsRegistry.java eac9f7295d968c3b30c40b89944e8f9f8549cc70
> giraph/src/main/java/org/apache/giraph/metrics/NoOpMetricsRegistry.java PRE-CREATION
> giraph/src/main/java/org/apache/giraph/metrics/package-info.java f7529f52527e8e4253f579bd6d01865aa980926b
>
> Diff: https://reviews.apache.org/r/7980/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nitay Joffe
>
>