date:20161220

[Impala-ASF-CR] IMPALA-3641: Fix catalogd RPC responses to DROP TABLE/DATABASE.

2016-12-20 Thread Alex Behm (Code Review)

Alex Behm has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5556

Change subject: IMPALA-3641: Fix catalogd RPC responses to DROP TABLE/DATABASE.
..

IMPALA-3641: Fix catalogd RPC responses to DROP TABLE/DATABASE.

The main problem was that the catalogd's response to a
DROP TABLE/DATABASE always included a removed object that was
applied to the requesting impalad's catalog cache.
In particular, a DROP IF EXISTS that did not actually drop
anything in the catalogd still returned the object name in
the RPC response as a removed object with the *current* catalog
version (i.e., without incrementing the catalog version).

The above behavior lead to a situation where a drop of
a non-existent object overwrote a legitimate entry in
an impalad's CatalogDeltaLog. Recall that the version of the
dropped object was based on the current catalog version
at some point in time, e.g., the same version of a
legitimate entry in the CatalogDeltaLog.

As a reminder, the CatalogDeltaLog protects deletions from
being undone via updates from the statestore. So overwriting
an object in the CatalogDeltaLog can lead to a dropped object
appearing again with certain timing of a statestore update.

Please see the JIRA for an analysis of logging output that
shows the bug and its effect.

The fix is simple: The RPC response of a DROP TABLE/DATABASE
should only contain a removed object if an object was actually
removed from the catalogd.

Testing:
- Unfortunately, I have not been able to reproduce the issue
  locally despite vigorous attempts and despite knowing what
  the problem is. Our existing tests seem to reproduce the
  issue pretty reliably, so it's not clear whether a targeted
  test is feasible or needed.
- An exhaustive test run passed.

Change-Id: Icb1f31eb2ecf05b9b51ef4e12e6bb78f44d0cf84
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 28 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/5556/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5556
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icb1f31eb2ecf05b9b51ef4e12e6bb78f44d0cf84
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm

[Impala-ASF-CR] IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions

2016-12-20 Thread Alex Behm (Code Review)

Alex Behm has posted comments on this change.

Change subject: IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5554/2/testdata/bin/check-hbase-nodes.py
File testdata/bin/check-hbase-nodes.py:

Line 143: LOGGER.warn("Zookeeper connection loss: retrying 
connection")
Might be worth logging the current connection attempt and the maximum attempts. 
(here or elsewhere)


Line 145: errors += 1
I suggest waiting a little before retrying to connect. How about 1s?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I44b4eec342addcfe489f94c332bbe14225c9968c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4702: Fix command line help for webserver private key file

2016-12-20 Thread Internal Jenkins (Code Review)

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4702: Fix command line help for 
webserver_private_key_file
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2820951b381240d5dab7c98e07e92332234462a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

2016-12-20 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change.

Change subject: IMPALA-4033: Treat string-partition key values as case 
sensitive.
..


Patch Set 4:

> > Thanks, Amos! I'll take care of running pre-commit tests and
 > > getting this patch merged.
 > 
 > Doing a pre-commit dry-run here:
 > 
 > http://jenkins.impala.io:8080/view/Gerrit/job/gerrit-verify-dryrun/131/

It passed

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fbe67d99df8a50a16a18456fde85d03d622c7a1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Amos Bird 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions

2016-12-20 Thread David Knupp (Code Review)

David Knupp has uploaded a new patch set (#2).

Change subject: IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions
..

IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions

This is the second patch to address IMPALA-4684. The first patch exposed
a transient Zookeeper connection error on RHEL7. This patch introduces a
retry (up to 3 times), and somewhat better logging.

Tested by running tests against an RHEL7 instance and confirming that
all HBase nodes start up.

Change-Id: I44b4eec342addcfe489f94c332bbe14225c9968c
---
M testdata/bin/check-hbase-nodes.py
1 file changed, 24 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/5554/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I44b4eec342addcfe489f94c332bbe14225c9968c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp

[Impala-ASF-CR] IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions

2016-12-20 Thread David Knupp (Code Review)

David Knupp has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5554

Change subject: IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions
..

IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions

This is the second patch to address IMPALA-4684. The first patch exposed
a transient Zookeeper connection error on RHEL7. This patch introduces a
retry (up to 3 times), and somewhat better logging.

Tested by running tests against an RHEL7 instance.

Change-Id: I44b4eec342addcfe489f94c332bbe14225c9968c
---
M testdata/bin/check-hbase-nodes.py
1 file changed, 24 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/5554/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I44b4eec342addcfe489f94c332bbe14225c9968c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp

[Impala-ASF-CR](asf-site) Change git links from GitHub to the official Apache repo.

2016-12-20 Thread Jim Apple (Code Review)

Jim Apple has submitted this change and it was merged.

Change subject: Change git links from GitHub to the official Apache repo.
..


Change git links from GitHub to the official Apache repo.

In the discussion on general@incubator around graduating Eagle, their
prominent GitHub links were a concern with some IPMC members.

Change-Id: Ifac93324dcd3344c8a32262749d2ac77632c4e5b
Reviewed-on: http://gerrit.cloudera.org:8080/5433
Reviewed-by: Tim Armstrong 
Tested-by: Jim Apple 
---
M bylaws.html
M community.html
M downloads.html
M impala-docs.html
M index.html
M overview.html
6 files changed, 56 insertions(+), 6 deletions(-)

Approvals:
  Jim Apple: Verified
  Tim Armstrong: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifac93324dcd3344c8a32262749d2ac77632c4e5b
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR](asf-site) Change git links from GitHub to the official Apache repo.

2016-12-20 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change.

Change subject: Change git links from GitHub to the official Apache repo.
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifac93324dcd3344c8a32262749d2ac77632c4e5b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4684: Wrap checking for HBase nodes ina try/finally block

2016-12-20 Thread Internal Jenkins (Code Review)

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4684: Wrap checking for HBase nodes ina  try/finally 
block
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I46a74d018f9169385a9f10a85718044c31a24dbc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4684: Wrap checking for HBase nodes ina try/finally block

2016-12-20 Thread Internal Jenkins (Code Review)

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4684: Wrap checking for HBase nodes ina  try/finally 
block
..


IMPALA-4684: Wrap checking for HBase nodes ina  try/finally block

If an exception (other than NoNodeError) was raised while checking for
HBase nodes, we weren't cleanly stopping the ZooKeeper client, which
in turn created a second exception when the the connection was closed.
The second exception masked the original error condition.

Tested by forcibly raising unexpected errors while checking for HBase
nodes.

Change-Id: I46a74d018f9169385a9f10a85718044c31a24dbc
Reviewed-on: http://gerrit.cloudera.org:8080/5547
Reviewed-by: Tim Armstrong 
Tested-by: Internal Jenkins
---
M testdata/bin/check-hbase-nodes.py
1 file changed, 4 insertions(+), 2 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Tim Armstrong: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I46a74d018f9169385a9f10a85718044c31a24dbc
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

2016-12-20 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change.

Change subject: IMPALA-4033: Treat string-partition key values as case 
sensitive.
..


Patch Set 4:

> Thanks, Amos! I'll take care of running pre-commit tests and
 > getting this patch merged.

Doing a pre-commit dry-run here:

http://jenkins.impala.io:8080/view/Gerrit/job/gerrit-verify-dryrun/131/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fbe67d99df8a50a16a18456fde85d03d622c7a1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Amos Bird 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: No

[Impala-ASF-CR](asf-site) Change git links from GitHub to the official Apache repo.

2016-12-20 Thread Jim Apple (Code Review)

Jim Apple has uploaded a new patch set (#2).

Change subject: Change git links from GitHub to the official Apache repo.
..

Change git links from GitHub to the official Apache repo.

In the discussion on general@incubator around graduating Eagle, their
prominent GitHub links were a concern with some IPMC members.

Change-Id: Ifac93324dcd3344c8a32262749d2ac77632c4e5b
---
M bylaws.html
M community.html
M downloads.html
M impala-docs.html
M index.html
M overview.html
6 files changed, 56 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/5433/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5433
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifac93324dcd3344c8a32262749d2ac77632c4e5b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4702: Fix command line help for webserver private key file

2016-12-20 Thread Tim Armstrong (Code Review)

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4702: Fix command line help for 
webserver_private_key_file
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2820951b381240d5dab7c98e07e92332234462a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4702: Fix command line help for webserver private key file

2016-12-20 Thread Lars Volker (Code Review)

Lars Volker has posted comments on this change.

Change subject: IMPALA-4702: Fix command line help for 
webserver_private_key_file
..


Patch Set 1:

(1 comment)

Thanks for the review. Can you carry the +2?

http://gerrit.cloudera.org:8080/#/c/5552/1//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4707: Fix command line help for webserver_private_key_file
> IMPALA-4702?
Good catch, thanks!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2820951b381240d5dab7c98e07e92332234462a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4702: Fix command line help for webserver private key file

2016-12-20 Thread Lars Volker (Code Review)

Hello Alex Behm,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/5552

to look at the new patch set (#2).

Change subject: IMPALA-4702: Fix command line help for 
webserver_private_key_file
..

IMPALA-4702: Fix command line help for webserver_private_key_file

The command line help for 'webserver_private_key_file' should refer to
'webserver_certificate_file' instead of 'ssl_server_certificate'.

Change-Id: Ie2820951b381240d5dab7c98e07e92332234462a
---
M be/src/util/webserver.cc
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/5552/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5552
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2820951b381240d5dab7c98e07e92332234462a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4707: Fix command line help for webserver private key file

2016-12-20 Thread Tim Armstrong (Code Review)

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4707: Fix command line help for 
webserver_private_key_file
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5552/1//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4707: Fix command line help for webserver_private_key_file
IMPALA-4702?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2820951b381240d5dab7c98e07e92332234462a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4707: Fix command line help for webserver private key file

2016-12-20 Thread Alex Behm (Code Review)

Alex Behm has posted comments on this change.

Change subject: IMPALA-4707: Fix command line help for 
webserver_private_key_file
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2820951b381240d5dab7c98e07e92332234462a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

2016-12-20 Thread Alex Behm (Code Review)

Alex Behm has posted comments on this change.

Change subject: IMPALA-4033: Treat string-partition key values as case 
sensitive.
..


Patch Set 4: Code-Review+2

Thanks, Amos! I'll take care of running pre-commit tests and getting this patch 
merged.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fbe67d99df8a50a16a18456fde85d03d622c7a1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Amos Bird 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4684: Wrap checking for HBase nodes ina try/finally block

2016-12-20 Thread Tim Armstrong (Code Review)

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4684: Wrap checking for HBase nodes ina  try/finally 
block
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I46a74d018f9169385a9f10a85718044c31a24dbc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3202,IMPALA-2079: rework scratch file I/O

2016-12-20 Thread Tim Armstrong (Code Review)

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3202,IMPALA-2079: rework scratch file I/O
..


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5141/16/be/src/runtime/buffered-block-mgr.cc
File be/src/runtime/buffered-block-mgr.cc:

Line 651:   status = RepinInMemBlock(block);
> That's not the case handled at line 624?
The handling of that case is split between the two places. The first case above 
is when it gets the buffer without trading in 'release_block', this case is 
when it needs to trade in 'release_block'. We could probably restructure the 
code to collapse the cases but it doesn't seem worth it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9c587df006d2f09d72dd636adafbd295fcdc17
Gerrit-PatchSet: 16
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3202,IMPALA-2079: rework scratch file I/O

2016-12-20 Thread Dan Hecht (Code Review)

Dan Hecht has posted comments on this change.

Change subject: IMPALA-3202,IMPALA-2079: rework scratch file I/O
..


Patch Set 20:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5141/16/be/src/runtime/buffered-block-mgr.cc
File be/src/runtime/buffered-block-mgr.cc:

Line 651:   }
> The case when the block is unpinned but the buffer is still in memory with 
That's not the case handled at line 624?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9c587df006d2f09d72dd636adafbd295fcdc17
Gerrit-PatchSet: 20
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4684: Wrap checking for HBase nodes ina try/finally block

2016-12-20 Thread David Knupp (Code Review)

David Knupp has posted comments on this change.

Change subject: IMPALA-4684: Wrap checking for HBase nodes ina  try/finally 
block
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5547/1/testdata/bin/check-hbase-nodes.py
File testdata/bin/check-hbase-nodes.py:

Line 139: errors = sum([check_znode(node, zk_client, timeout) for node 
in nodes])
> It might be cleaner to use a try/finally here so we stop it in the same pla
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I46a74d018f9169385a9f10a85718044c31a24dbc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

2016-12-20 Thread Tim Armstrong (Code Review)

Hello Dan Hecht,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/5545

to look at the new patch set (#2).

Change subject: IMPALA-4653: fix sticky config variable problem
..

IMPALA-4653: fix sticky config variable problem

Previously we could get a developer's shell into a bad state where a
value of a config variable from a previous impala-config.sh version
would override the value from the new impala-config.sh version.

This change adds a new mechanism to override settings locally by adding
settings to impala-config-local.sh. This alternative approach is more
robust, because the config variables will be reset to the intended
values when impala-config.sh is re-sourced.

impala-config-branch.sh can also be used to override settings in a
version-controlled way, e.g. to support having different settings for
different branches.

I did not convert all variables to use this approach, since many people
and Jenkins jobs depend on setting these variables from the environment.
The remaining "sticky" variables are ones where default values should
not change frequently, e.g. source directory locations and build
settings.

Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
---
M .gitignore
A bin/impala-config-branch.sh
M bin/impala-config.sh
3 files changed, 116 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/5545/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I930e2ca825142428d17a6981c77534ab0c8e3489
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht

[Impala-ASF-CR] IMPALA-4684: Handle unexpected exceptions while checking for HBase nodes

2016-12-20 Thread Tim Armstrong (Code Review)

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4684: Handle unexpected exceptions while checking for 
HBase nodes
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5547/1/testdata/bin/check-hbase-nodes.py
File testdata/bin/check-hbase-nodes.py:

Line 139: errors = sum([check_znode(node, zk_client, timeout) for node 
in nodes])
It might be cleaner to use a try/finally here so we stop it in the same place 
on both paths.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I46a74d018f9169385a9f10a85718044c31a24dbc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3202,IMPALA-2079: rework scratch file I/O

2016-12-20 Thread Tim Armstrong (Code Review)

Tim Armstrong has uploaded a new patch set (#20).

Change subject: IMPALA-3202,IMPALA-2079: rework scratch file I/O
..

IMPALA-3202,IMPALA-2079: rework scratch file I/O

Refactor BufferedBlockMgr/TmpFileMgr to push more I/O logic into
TmpFileMgr, in anticipation of it being shared with BufferPool.
TmpFileMgr now handles:
* Scratch space allocation and recycling
* Read and write I/O

The interface is also greatly changed so that it is built around Write()
and Read() calls, abstracting away the details of temporary file
allocation from clients. This means the TmpFileMgr::File class can
be hidden from clients.

Write error recovery:
Also implement write error recovery in TmpFileMgr.

If an error occurs while writing to scratch and we have multiple
scratch directories, we will try one of the other directories
before cancelling the query. File-level blacklisting is used to
prevent excessive repeated attempts to resize a scratch file during
a single query. Device-level blacklisting is still disabled because
it is problematic to permanently take a scratch directory out of use.

To reduce the number of error paths, all I/O errors are now handled
asynchronously. Previously errors creating or extending the file were
returned synchronously from WriteUnpinnedBlock(). This required
modifying DiskIoMgr to create the file if not present when opened.

Also set the default max_errors value in the thrift definition file,
so that it is in effect for backend tests.

Future Work:
* Support for recycling variable-length scratch file ranges. I omitted
  this to avoid making the patch even large.

Testing:
Updated BufferedBlockMgr unit test to reflect changes in behaviour:
* Scratch space is no longer permanently associated with a block, and
  is remapped every time a new block is written to disk .
* Files are now blacklisted - updated existing tests and enable the
  disable blacklisting test.

Added some basic testing of recycling of scratch file ranges in
the TmpFileMgr unit test.

I also manually tested the code in two ways. First by removing permissions
for /tmp/impala-scratch and ensuring that a spilling query fails cleanly.
Second, by creating a tiny ramdisk (16M) and running with two scratch
directories: one on /tmp and one on the tiny ramdisk. When spilling, an
out of space error is encountered for the tiny ramdisk and impala spills
the remaining data (72M) to /tmp.

Change-Id: I8c9c587df006d2f09d72dd636adafbd295fcdc17
---
M be/src/runtime/buffered-block-mgr-test.cc
M be/src/runtime/buffered-block-mgr.cc
M be/src/runtime/buffered-block-mgr.h
M be/src/runtime/disk-io-mgr-test.cc
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/disk-io-mgr.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/query-state.cc
A be/src/runtime/tmp-file-mgr-internal.h
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
A be/src/util/buffer-ref.h
M be/src/util/disk-info.h
M be/src/util/filesystem-util.cc
M be/src/util/filesystem-util.h
M common/thrift/ImpalaInternalService.thrift
18 files changed, 1,279 insertions(+), 625 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/5141/20
-- 
To view, visit http://gerrit.cloudera.org:8080/5141
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9c587df006d2f09d72dd636adafbd295fcdc17
Gerrit-PatchSet: 20
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-3202,IMPALA-2079: rework scratch file I/O

2016-12-20 Thread Tim Armstrong (Code Review)

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3202,IMPALA-2079: rework scratch file I/O
..


Patch Set 17:

(22 comments)

http://gerrit.cloudera.org:8080/#/c/5141/16/be/src/runtime/buffered-block-mgr.cc
File be/src/runtime/buffered-block-mgr.cc:

Line 651:   status = RepinInMemBlock(block);
> why do we need to do this since the block wasn't in memory in this case?
The case when the block is unpinned but the buffer is still in memory with the 
data - so we don't need to do any I/O but may need to destroy the write handle 
and decrypt the data.


Line 669: if (!status.ok()) goto error;
> if the read failed, why don't we have to still CloseWrite()?
It gets cleaned up when the block is deleted in that case (same if we hit an 
error in TransferBuffer(), etc).


Line 687: WaitForWrite(lock, block);
I realised I should also check for cancellation here.


Line 691: if (block->valid_data_len() == 0) {
> when is the 0 len case used?
I ended up removing the special case.


Line 751:   // Workaround to avoid adding a special code path for the 0-byte 
edge case.
> what is that special case? can we not issue writes of zero length to diskio
BufferPool won't need this since it doesn't have the valid_data_len_ stuff and 
will just flush the whole buffer to disk.

Zero-length writes should work but seem like an easy thing to break. I removed 
the special case.


http://gerrit.cloudera.org:8080/#/c/5141/16/be/src/runtime/buffered-block-mgr.h
File be/src/runtime/buffered-block-mgr.h:

PS16, Line 403: evicted from
  :   /// memory
> I'm confused by the name "InMemBlock" given this says it handles blocks tha
Yeah this was inaccurate. Rewrote to make it clear that it's a block that was 
written to disk or has an in-flight write.


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

Line 868:   /// not associated with a particular local disk or remote queue). 
Use to implement
> weird line break
Done


http://gerrit.cloudera.org:8080/#/c/5141/16/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

Line 49
> also not sure if you meant to include this?
I ran into the default max_errors value problem and saw this code was stale. 
The flag doesn't exist and we have a better way to set default query options. 
Thought it was best to just delete it.


http://gerrit.cloudera.org:8080/#/c/5141/16/be/src/runtime/tmp-file-mgr-internal.h
File be/src/runtime/tmp-file-mgr-internal.h:

Line 28: /// can be allocated and files removed using AllocateSpace() and 
Remove().
> let's clarify that File should only used internally TmpFileMgr
Done


PS16, Line 41: offset
> .. to the location of the newly allocated file space?
Done


PS16, Line 55:  it is a remote FS, disk IDs are chosen round-robin.
> does this preserve existing behavior for temporary files? for data files, w
Yeah this preserved the old behaviour, which was copy-pasted from DiskIoMgr. I 
switched to using the DiskIoMgr implementation because there wasn't any reason 
to duplicate the logic.


PS16, Line 65: created
> creates?
Done


Line 83:   const int disk_id_;
> do we need to keep DeviceId around, or could we just use disk_ids?
We could, but then we'd have to maintain a sparse map of 'disk_id' -> 
'device_id'. And it would break all of the backend tests that rely on being 
able to have multiple temporary directories per physical disk.


http://gerrit.cloudera.org:8080/#/c/5141/16/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:

PS16, Line 150: 
> sometimes we pass as const&, other times by value. Is there a reason for th
Nope. Changed it to just pass by value. This is the only case I could find.

I think this was probably a result of over-zealous application of the "always 
pass classes by const ref" rule.


PS16, Line 209: 
> ActiveTmpDevices
Done


PS16, Line 394: 
  : 
> given that we now accumulate errors, why say this?
It looks like there are some cases where the error logging mechanism ignores 
the merged status messages. I filed a JIRA about that: IMPALA-4697. Added a 
TODO here to fix that.


PS16, Line 431: 
  : bytes_rea
> that's always true. is this trying to say that the Read() is synchronous an
Yes - clarified it.


http://gerrit.cloudera.org:8080/#/c/5141/16/be/src/runtime/tmp-file-mgr.h
File be/src/runtime/tmp-file-mgr.h:

Line 76: 
> do external classes still care about DeviceId's, now that File's aren't exp
It's only useful for TmpFileMgrTest now - added a comment to explain.


Line 125: /// Synchronously read the data referenced by 'handle' from the 
temporary file into
> Yeah, maybe DestroyWriteHandle()?
Done


http://gerrit.cloudera.org:8080/#/c/5141/19/be/src/runtime/tmp-file-mgr.h
File be/src/runtime/tmp-file-mgr.h:

PS19, Line 249: and
> then?
Done


Line 250:   ///
> i think it would be good to explain the lifecycle

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

2016-12-20 Thread Amos Bird (Code Review)

Hello Alex Behm,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/5535

to look at the new patch set (#4).

Change subject: IMPALA-4033: Treat string-partition key values as case 
sensitive.
..

IMPALA-4033: Treat string-partition key values as case sensitive.

This commit makes ADD PARTITION operations treat string partition-key
values as case sensitive in consistent with other related partition DDL
operations.

Change-Id: I6fbe67d99df8a50a16a18456fde85d03d622c7a1
---
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M 
testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates-all-fs.test
4 files changed, 25 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/5535/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5535
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6fbe67d99df8a50a16a18456fde85d03d622c7a1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Amos Bird 
Gerrit-Reviewer: Jim Apple

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

2016-12-20 Thread Alex Behm (Code Review)

Alex Behm has posted comments on this change.

Change subject: IMPALA-4033: Treat string-partition key values as case 
sensitive.
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5535/3/testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates-all-fs.test
File 
testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates-all-fs.test:

Line 137: alter table p1 add partition (j=2,k="D");
# Tests case-sensitivity of string-typed partition columns.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fbe67d99df8a50a16a18456fde85d03d622c7a1
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Amos Bird 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3641: Fix catalogd RPC responses to DROP TABLE/DATABASE.

[Impala-ASF-CR] IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions

[Impala-ASF-CR] IMPALA-4702: Fix command line help for webserver private key file

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

[Impala-ASF-CR] IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions

[Impala-ASF-CR] IMPALA-4684: Handle Zookeeper ConnentionLoss exceptions

[Impala-ASF-CR](asf-site) Change git links from GitHub to the official Apache repo.

[Impala-ASF-CR](asf-site) Change git links from GitHub to the official Apache repo.

[Impala-ASF-CR] IMPALA-4684: Wrap checking for HBase nodes ina try/finally block

[Impala-ASF-CR] IMPALA-4684: Wrap checking for HBase nodes ina try/finally block

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

[Impala-ASF-CR](asf-site) Change git links from GitHub to the official Apache repo.

[Impala-ASF-CR] IMPALA-4702: Fix command line help for webserver private key file

[Impala-ASF-CR] IMPALA-4702: Fix command line help for webserver private key file

[Impala-ASF-CR] IMPALA-4702: Fix command line help for webserver private key file

[Impala-ASF-CR] IMPALA-4707: Fix command line help for webserver private key file

[Impala-ASF-CR] IMPALA-4707: Fix command line help for webserver private key file

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

[Impala-ASF-CR] IMPALA-4684: Wrap checking for HBase nodes ina try/finally block

[Impala-ASF-CR] IMPALA-3202,IMPALA-2079: rework scratch file I/O

[Impala-ASF-CR] IMPALA-3202,IMPALA-2079: rework scratch file I/O

[Impala-ASF-CR] IMPALA-4684: Wrap checking for HBase nodes ina try/finally block

[Impala-ASF-CR] IMPALA-4653: fix sticky config variable problem

[Impala-ASF-CR] IMPALA-4684: Handle unexpected exceptions while checking for HBase nodes

[Impala-ASF-CR] IMPALA-3202,IMPALA-2079: rework scratch file I/O

[Impala-ASF-CR] IMPALA-3202,IMPALA-2079: rework scratch file I/O

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

[Impala-ASF-CR] IMPALA-4033: Treat string-partition key values as case sensitive.

28 matches

Site Navigation

Mail list logo

Footer information