Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201705262244.v4QMijgc027264@ip-10-146-233-104.ec2.internal>
Date: Fri, 26 May 2017 22:44:45 +0000
From: "Joe McDonnell (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
CC: Tim Armstrong <tarmstrong@cloudera.com>,
        Jim Apple <jbapple-impala@apache.org>,
        Marcel Kornacker <marcel@cloudera.com>,
        Dan Hecht <dhecht@cloudera.com>
Reply-To: joemcdonnell@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4623=3A_Enable_file_handle_cache=0A?=
In-Reply-To: <gerrit.1490402401000.Ibe5ff60971dd653c3b6a0e13928cfa9fc59d078d@gerrit.cloudera.org>
References: <gerrit.1490402401000.Ibe5ff60971dd653c3b6a0e13928cfa9fc59d078d@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.7
archived-at: Fri, 26 May 2017 22:44:49 -0000

Joe McDonnell has posted comments on this change.

Change subject: IMPALA-4623: Enable file handle cache
......................................................................


Patch Set 22:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6478/22/be/src/runtime/disk-io-mgr-handle-cache.h
File be/src/runtime/disk-io-mgr-handle-cache.h:

Line 155:     uint8_t padding[CACHE_LINE_SIZE];
> What happened? The clang-tidy complaints seem to be about other types; does
I misunderstood and thought that changing the alignas(64) on this struct to a CacheLineAligned would eliminate the problem, but aligning this in either way requires DiskIoMgr to also be aligned.


http://gerrit.cloudera.org:8080/#/c/6478/22/be/src/runtime/disk-io-mgr.h
File be/src/runtime/disk-io-mgr.h:

Line 199: class DiskIoMgr {
> This is apparently a type that should be cache-aligned.
This cache alignment is required because FileHandleCachePartition is cache aligned. To pass clang tidy, cache alignment should apply to both or neither. This upload does neither. I'm switching it to both.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe5ff60971dd653c3b6a0e13928cfa9fc59d078d
Gerrit-PatchSet: 22
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes