[ 
https://issues.apache.org/jira/browse/HADOOP-12824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15163449#comment-15163449
 ] 

Xiaoyu Yao commented on HADOOP-12824:
-------------------------------------

[~elgoiri], thanks for working on this. The patch looks good overall, a few 
comments below:

1. The title needs to be updated to match with the patch, which collects both 
total network and disk usage.

2.  Potential memory leak in ReadTotalCounter() due to the direct return below. 
Suggest to bailout with goto to ensure pItems is freed before the multiple 
returns in this function.
{code}
status = PdhGetRawCounterArray(hCounter, &dwBufferSize, &dwItemCount, NULL);
248       if (PDH_MORE_DATA == status)
249       {
250         pItems = (PDH_RAW_COUNTER_ITEM *) malloc(dwBufferSize);
251         if (pItems)
252         {
253           // Actually query the counter
254           status = PdhGetRawCounterArray(hCounter, &dwBufferSize, 
&dwItemCount, pItems);
255           if (ERROR_SUCCESS == status) {
256             for (i = 0; i < dwItemCount; i++) {
257               if (wcscmp(L"_Total", pItems[i].szName) == 0) {
258                 totalFound = 1;
259                 *ret = pItems[i].RawValue.FirstValue;
260               } else if (!totalFound) {
261                 *ret += pItems[i].RawValue.FirstValue;
262               }
263             }
264           } else {
265             return status;
266           }
{code}

3. I see usage of  SAL2 style annotations in the second parameter of 
ReadTotalCounter() but not other new functions added. I suggest we should use 
it consistently.
{code}
        PDH_STATUS ReadTotalCounter(PDH_HCOUNTER hCounter, _Out_ LONGLONG* ret)
{code}

> Collect network usage on the node in Windows
> --------------------------------------------
>
>                 Key: HADOOP-12824
>                 URL: https://issues.apache.org/jira/browse/HADOOP-12824
>             Project: Hadoop Common
>          Issue Type: Improvement
>    Affects Versions: 2.8.0
>            Reporter: Inigo Goiri
>            Assignee: Inigo Goiri
>         Attachments: HADOOP-12824-v000.patch, HADOOP-12824-v001.patch
>
>
> HADOOP-12210 collects the node network usage for Linux; this JIRA does it for 
> Windows.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to