From codereview-return-6923-apmail-trafodion-codereview-archive=trafodion.apache.org@trafodion.apache.org  Wed May  9 21:19:53 2018
Return-Path: <codereview-return-6923-apmail-trafodion-codereview-archive=trafodion.apache.org@trafodion.apache.org>
X-Original-To: apmail-trafodion-codereview-archive@minotaur.apache.org
Delivered-To: apmail-trafodion-codereview-archive@minotaur.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 1DB8818D80
	for <apmail-trafodion-codereview-archive@minotaur.apache.org>; Wed,  9 May 2018 21:19:53 +0000 (UTC)
Received: (qmail 1174 invoked by uid 500); 9 May 2018 21:19:52 -0000
Delivered-To: apmail-trafodion-codereview-archive@trafodion.apache.org
Received: (qmail 1144 invoked by uid 500); 9 May 2018 21:19:52 -0000
Mailing-List: contact codereview-help@trafodion.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:codereview-help@trafodion.apache.org>
List-Unsubscribe: <mailto:codereview-unsubscribe@trafodion.apache.org>
List-Post: <mailto:codereview@trafodion.apache.org>
List-Id: <codereview.trafodion.apache.org>
Reply-To: codereview@trafodion.apache.org
Delivered-To: mailing list codereview@trafodion.apache.org
Received: (qmail 99901 invoked by uid 99); 9 May 2018 21:19:52 -0000
Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 09 May 2018 21:19:52 +0000
Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33)
	id AB42CF6CAA; Wed,  9 May 2018 21:19:51 +0000 (UTC)
From: DaveBirdsall <git@git.apache.org>
To: codereview@trafodion.apache.org
Reply-To: codereview@trafodion.apache.org
References: <git-pr-1557-trafodion@git.apache.org>
In-Reply-To: <git-pr-1557-trafodion@git.apache.org>
Subject: [GitHub] trafodion pull request #1557: Changes for [TRAFODION-3065] and [TRAFODION-29...
Content-Type: text/plain
Message-Id: <20180509211951.AB42CF6CAA@git1-us-west.apache.org>
Date: Wed,  9 May 2018 21:19:51 +0000 (UTC)

Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1557#discussion_r187178015
  
    --- Diff: core/sql/executor/HdfsClient_JNI.cpp ---
    @@ -574,41 +595,50 @@ Int32 HdfsClient::hdfsWrite(const char* data, Int64 len, HDFS_Client_RetCode &hd
          hdfsClientRetcode = HDFS_CLIENT_ERROR_HDFS_WRITE_EXCEPTION;
          return 0;
       }
    -
    -  //Write the requisite bytes into the file
    -  jbyteArray jbArray = jenv_->NewByteArray( len);
    -  if (!jbArray) {
    -    GetCliGlobals()->setJniErrorStr(getErrorText(HDFS_CLIENT_ERROR_HDFS_WRITE_PARAM));
    -    jenv_->PopLocalFrame(NULL);
    -    hdfsClientRetcode =  HDFS_CLIENT_ERROR_HDFS_WRITE_PARAM;
    -    return 0;
    -  }
    -  jenv_->SetByteArrayRegion(jbArray, 0, len, (const jbyte*)data);
    -
    -  if (hdfsStats_ != NULL)
    -     hdfsStats_->getHdfsTimer().start();
    -
    -  tsRecentJMFromJNI = JavaMethods_[JM_HDFS_WRITE].jm_full_name;
    -  // Java method returns the cumulative bytes written
    -  jint totalBytesWritten = jenv_->CallIntMethod(javaObj_, JavaMethods_[JM_HDFS_WRITE].methodID, jbArray);
    -
    -  if (hdfsStats_ != NULL) {
    -      hdfsStats_->incMaxHdfsIOTime(hdfsStats_->getHdfsTimer().stop());
    -      hdfsStats_->incHdfsCalls();
    -  }
    -  if (jenv_->ExceptionCheck())
    +  Int64 lenRemain = len;
    +  Int64 writeLen;
    +  Int64 chunkLen = (ioByteArraySize_ > 0 ? ioByteArraySize_ * 1024 : 0);
    +  Int64 offset = 0;
    +  do 
       {
    -    getExceptionDetails(__FILE__, __LINE__, "HdfsClient::hdfsWrite()");
    -    jenv_->PopLocalFrame(NULL);
    -    hdfsClientRetcode = HDFS_CLIENT_ERROR_HDFS_WRITE_EXCEPTION;
    -    return 0;
    -  }
    -
    +     if ((chunkLen > 0) && (lenRemain > chunkLen))
    +        writeLen = chunkLen; 
    +     else
    +        writeLen = lenRemain;
    +     //Write the requisite bytes into the file
    +     jbyteArray jbArray = jenv_->NewByteArray(writeLen);
    +     if (!jbArray) {
    +        GetCliGlobals()->setJniErrorStr(getErrorText(HDFS_CLIENT_ERROR_HDFS_WRITE_PARAM));
    +        jenv_->PopLocalFrame(NULL);
    +        hdfsClientRetcode =  HDFS_CLIENT_ERROR_HDFS_WRITE_PARAM;
    +        return 0;
    +     }
    +     jenv_->SetByteArrayRegion(jbArray, 0, writeLen, (const jbyte*)(data+offset));
    +
    +     if (hdfsStats_ != NULL)
    +         hdfsStats_->getHdfsTimer().start();
    +
    +     tsRecentJMFromJNI = JavaMethods_[JM_HDFS_WRITE].jm_full_name;
    +     // Java method returns the cumulative bytes written
    +     jint totalBytesWritten = jenv_->CallIntMethod(javaObj_, JavaMethods_[JM_HDFS_WRITE].methodID, jbArray);
    +
    +     if (hdfsStats_ != NULL) {
    +         hdfsStats_->incMaxHdfsIOTime(hdfsStats_->getHdfsTimer().stop());
    +         hdfsStats_->incHdfsCalls();
    +     }
    +     if (jenv_->ExceptionCheck())
    +     {
    +        getExceptionDetails(__FILE__, __LINE__, "HdfsClient::hdfsWrite()");
    +        jenv_->PopLocalFrame(NULL);
    +        hdfsClientRetcode = HDFS_CLIENT_ERROR_HDFS_WRITE_EXCEPTION;
    +        return 0;
    +     }
    +     lenRemain -= writeLen;
    +     offset += writeLen;
    +  } while (lenRemain > 0);
    --- End diff --
    
    What happens if len is initially zero or negative? (Usually one codes a "while" instead of "do while" so that zero cases are harmless).


---