[ 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)