Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201609282328.u8SNSRGo018293@ip-10-146-233-104.ec2.internal>
Date: Wed, 28 Sep 2016 23:28:27 +0000
From: "Alex Behm (Code Review)" <gerrit@cloudera.org>
To: Tim Armstrong <tarmstrong@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Michael Ho <kwho@cloudera.com>
Reply-To: alex.behm@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4118=3A_extract_encryption_utils_from_BufferedBlockMgr=0A?=
In-Reply-To: <gerrit.1473723985000.Ibebe431f69c3c569cbff68171beaa32ef2ab03b6@gerrit.cloudera.org>
References: <gerrit.1473723985000.Ibebe431f69c3c569cbff68171beaa32ef2ab03b6@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Wed, 28 Sep 2016 23:28:32 -0000

Alex Behm has posted comments on this change.

Change subject: IMPALA-4118: extract encryption utils from BufferedBlockMgr
......................................................................


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4389/6/be/src/util/openssl-util.cc
File be/src/util/openssl-util.cc:

Line 78:     SeedOpenSSLRNG();
> The RNG is global. Was the concern performance?
Concern was not really about perf, but about making the behavior as simple/understandable as possible.
If the RNG was thread-local some threads might never reseed their RNG because they never happen to hit the right global counter value, so that would have been wrong.

A global RNG and a global counter seem correct and easy to understand, so I'm happy.

Might be worth adding a comment somewhere that the RNG is global.


http://gerrit.cloudera.org:8080/#/c/4389/6/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

Line 35:  public:
> I thought about it and it seemed mostly not useful, since verification agai
How can you be sure the verification will fail? It seems like there's a remote chance that the hash_ will be initialized to a value that coincidentally matches the hash of the data you are passing in Verify(), but not the hash that would have been computed in Compute(). That chance even exists if we explicitly zero out hash_ or initialise it in some other way.


-- 
To view, visit http://gerrit.cloudera.org:8080/4389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibebe431f69c3c569cbff68171beaa32ef2ab03b6
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes