You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "Patrick Wendell (JIRA)" <ji...@apache.org> on 2010/06/23 23:37:52 UTC
[jira] Created: (AVRO-584) Update Histogram for Stats Plugin
Update Histogram for Stats Plugin
---------------------------------
Key: AVRO-584
URL: https://issues.apache.org/jira/browse/AVRO-584
Project: Avro
Issue Type: Sub-task
Reporter: Patrick Wendell
Assignee: Patrick Wendell
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Patrick Wendell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12882069#action_12882069 ]
Patrick Wendell commented on AVRO-584:
--------------------------------------
The problem is in how the bucket iterator is built. As the code is now, when instantiated it increments the internal iterator over keys. That internal iterator is used to evaluate the hasNext() call... which is then off-by-one. Was this intentional? I can just change that code too...
{quote}
Iterator<T> it = index.keySet().iterator();
T cur = it.next(); // there's always at least one element
...
@Override
public boolean hasNext() {
return it.hasNext();
}
{quote}
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
> Attachments: AVRO-584.patch, AVRO-584.patch.v2.txt
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Patrick Wendell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Wendell updated AVRO-584:
---------------------------------
Attachment: AVRO-584.patch.v2.txt
Addressing feedback from review.
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
> Attachments: AVRO-584.patch, AVRO-584.patch.v2.txt
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Philip Zeyliger resolved AVRO-584.
----------------------------------
Hadoop Flags: [Reviewed]
Resolution: Fixed
I comitted this as r957730. I removed a println statement from TestHistograms.java.
Thanks for the contribution, Patrick!
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
> Attachments: AVRO-584.patch, AVRO-584.patch.v2.txt, AVRO-584.patch.v3.txt
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Patrick Wendell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Wendell updated AVRO-584:
---------------------------------
Attachment: AVRO-584.patch.v3.txt
Okay just changed the implementation of the iterator so it follows the correct convention.
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
> Attachments: AVRO-584.patch, AVRO-584.patch.v2.txt, AVRO-584.patch.v3.txt
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881951#action_12881951 ]
Philip Zeyliger commented on AVRO-584:
--------------------------------------
I posted this to reviewboard and took a look there. My comments are pasted in below. I'm curious to see how stats plugin is going to use the histogram data for recent stuff. It seems like it might be wiser to keep that data next to, but outside, of the histogram. Consider it.
Thanks!
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/225/#review272
-----------------------------------------------------------
A couple of nits on the documentation, but otherwise looks great. When you create your next patch, use --no-prefix in "git diff" to strip out the a/ and b/. SVN folks prefer that.
lang/java/src/java/org/apache/avro/ipc/stats/Histogram.java
<http://review.hbase.org/r/225/#comment1134>
If it's public, you may as well make your comment a javadoc.
lang/java/src/java/org/apache/avro/ipc/stats/Histogram.java
<http://review.hbase.org/r/225/#comment1135>
I believe this is spelled "boundary" with plural "boundaries".
lang/java/src/java/org/apache/avro/ipc/stats/Histogram.java
<http://review.hbase.org/r/225/#comment1136>
Mention that this keeps the last MAX_HISTORY_SIZE entries. Are those recent entries in order? If so, you should mention whether you intend to keep that promise in the javadoc.
- Philip
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
> Attachments: AVRO-584.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881945#action_12881945 ]
Philip Zeyliger commented on AVRO-584:
--------------------------------------
Thanks for the patch; I'll take a look!
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
> Attachments: AVRO-584.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Patrick Wendell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881899#action_12881899 ]
Patrick Wendell commented on AVRO-584:
--------------------------------------
We need to make a few changes to the Histogram in order for the stats plugin to work correctly. I'm committing these separately to make reviews more manageable size.
1. Add IntegerHistogram to track payload data
2. Create functions to display the bucket labels easily when graphing
3. Track recent additions to the histogram (this is used by Stats Plugin to give data on recent RPCs)
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Patrick Wendell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Wendell updated AVRO-584:
---------------------------------
Attachment: AVRO-584.patch
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
> Attachments: AVRO-584.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-584) Update Histogram for Stats Plugin
Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12882057#action_12882057 ]
Philip Zeyliger commented on AVRO-584:
--------------------------------------
Quick question:
{quote}
+ Iterator<String> bucketsIt = this.getBuckets();
+ do {
+ outArray.add(bucketsIt.next());
+ } while (bucketsIt.hasNext());
+ outArray.add(bucketsIt.next());
{quote}
That looks a bit wrong, because you're calling bucketsIt.next() after bucketsIt.hasNext() has returned false. Am I missing something?
> Update Histogram for Stats Plugin
> ---------------------------------
>
> Key: AVRO-584
> URL: https://issues.apache.org/jira/browse/AVRO-584
> Project: Avro
> Issue Type: Sub-task
> Reporter: Patrick Wendell
> Assignee: Patrick Wendell
> Attachments: AVRO-584.patch, AVRO-584.patch.v2.txt
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.