hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Patrick McCabe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-4712) New libhdfs method hdfsGetDataNodes
Date Tue, 23 Jul 2013 17:42:49 GMT

    [ https://issues.apache.org/jira/browse/HDFS-4712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13716650#comment-13716650

Colin Patrick McCabe commented on HDFS-4712:

+    if (jVal.l == NULL) 
+        strcpy(dataNodeInfos->hostName,"");
+    else 
+        jExc =  newCStr(env, jVal.l, &dataNodeInfos->hostName);

You want {{dataNodeInfos->hostName = strdup("")}} here

+    typedef struct  hdfsDataNodeInfo_s {
+        long capacity;        /* The raw capacity */
+        long dfsUsed;         /* The used space by the data node*/
+        char *hostName;       /* HostName as supplied by the datanode during registration
as its name*/
+        tTime lastUpdate;     /* The time when this information was accurate */
+        char *location;       /* The rack name */
+        long remaining;       /* The raw free space */
+        short xceiverCount;   /* The  number of active connections */
+    } hdfsDataNodeInfo;

Please use {{int64_t}} instead of {{long}} here.  {{long}} has different sizes on different
architectures, but these quantities should always be 64-bit.  I think it makes more sense
to use {{uint32_t}} for xceiverCount, since I don't know if "32767 will be enough for anyone."

+         if (hdfsDataNodeInfos[i]->hostName) {
+            free(hdfsDataNodeInfos[i]->hostName);
+          }
+         if (hdfsDataNodeInfos[i]->location) {
+            free(hdfsDataNodeInfos[i]->location);
+        }
You don't need to test if something is NULL before calling {{free}} on it.  {{free}} ignores
NULL pointers.

{{hdfsGetDataNodeInfo}}: the error handling is wrong here.  You should return NULL when there
is an error.  You must also free the memory you've allocated on this line:
+    dataNodeInfoList = calloc(jNumDataNodeInfos, sizeof(hdfsDataNodeInfo*));

It is preferrable to use a pattern like this:
  if (bad thing) {
    ret = EIO;
    goto done;

  if (ret) {
    resources = NULL;
    errno = ret;
  return resources;

This way:
* You always free the resources on error.
* setting errno is the last thing you do, and won't be overwritten by another function call.
* You return NULL on error rather than a bogus pointer.

> New libhdfs method hdfsGetDataNodes
> -----------------------------------
>                 Key: HDFS-4712
>                 URL: https://issues.apache.org/jira/browse/HDFS-4712
>             Project: Hadoop HDFS
>          Issue Type: New Feature
>          Components: libhdfs
>            Reporter: andrea manzi
>         Attachments: HDFS-4712.patch, hdfs.c.diff, hdfs.h.diff
> we have implemented a possible extension to libhdfs to retrieve information about the
available datanodes ( there was a mail on the hadoop-hdsf-dev mailing list initially abut
this :
> http://mail-archives.apache.org/mod_mbox/hadoop-hdfs-dev/201204.mbox/%3CCANhO-
> s0mvORORrxPjnjbQL6bRkJ4C7L+u816xkdc+2r0WHjBcw@mail.gmail.com%3E)
> i would like to know how to proceed to create a patch, cause on the wiki http://wiki.apache.org/hadoop/HowToContribute
i can see info about JAVA patches but nothing related to extensions in C.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

View raw message