You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2023/03/02 17:46:58 UTC
[impala] branch master updated (eaf71bca0 -> 1321b5ce5)
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
from eaf71bca0 IMPALA-11948: Remove operation logging from getCatalogServerMetrics
new 8b375a66a IMPALA-11565: Support IF NOT EXISTS in alter table add columns for kudu/iceberg table
new 99d676f8f IMPALA-11920: Support spill to HDFS address by service ID
new 1321b5ce5 IMPALA-11920: [DOCS] Cleanup and update spill examples
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/runtime/tmp-file-mgr-test.cc | 72 ++++++----
be/src/runtime/tmp-file-mgr.cc | 22 ++-
common/thrift/JniCatalog.thrift | 2 +-
docs/topics/impala_disk_space.xml | 153 +++++++++++++--------
.../impala/analysis/AlterTableAddColsStmt.java | 17 ++-
.../apache/impala/service/CatalogOpExecutor.java | 36 +++--
.../apache/impala/analysis/AnalyzeKuduDDLTest.java | 5 +
.../queries/QueryTest/iceberg-alter.test | 57 ++++++++
.../queries/QueryTest/kudu_alter.test | 92 +++++++++++++
9 files changed, 349 insertions(+), 107 deletions(-)
[impala] 01/03: IMPALA-11565: Support IF NOT EXISTS in alter table add columns for kudu/iceberg table
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 8b375a66a29cfea33a4e418b1fa3b9b5139cf907
Author: xiabaike <xi...@163.com>
AuthorDate: Thu Sep 8 09:18:50 2022 +0000
IMPALA-11565: Support IF NOT EXISTS in alter table add columns for kudu/iceberg table
Impala already supports IF NOT EXISTS in alter table add columns for
general hive table in IMPALA-7832, but not for kudu/iceberg table.
This patch try to add such semantics for kudu/iceberg table.
Testing:
- Updated E2E DDL tests
- Added fe tests
Change-Id: I82590e5372e881f2e81d4ed3dd0d32a2d3ddb517
Reviewed-on: http://gerrit.cloudera.org:8080/18953
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
---
common/thrift/JniCatalog.thrift | 2 +-
.../impala/analysis/AlterTableAddColsStmt.java | 17 +++-
.../apache/impala/service/CatalogOpExecutor.java | 36 ++++++---
.../apache/impala/analysis/AnalyzeKuduDDLTest.java | 5 ++
.../queries/QueryTest/iceberg-alter.test | 57 ++++++++++++++
.../queries/QueryTest/kudu_alter.test | 92 ++++++++++++++++++++++
6 files changed, 194 insertions(+), 15 deletions(-)
diff --git a/common/thrift/JniCatalog.thrift b/common/thrift/JniCatalog.thrift
index 3efe0d0e5..c41762a6f 100755
--- a/common/thrift/JniCatalog.thrift
+++ b/common/thrift/JniCatalog.thrift
@@ -222,7 +222,7 @@ struct TAlterTableOrViewRenameParams {
// Parameters for ALTER TABLE ADD COLUMNS commands.
struct TAlterTableAddColsParams {
// List of columns to add to the table
- 1: required list<CatalogObjects.TColumn> columns
+ 1: optional list<CatalogObjects.TColumn> columns
// If true, no error is raised when a column already exists.
2: required bool if_not_exists
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
index e373e9fa3..dece3c4f8 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableAddColsStmt.java
@@ -32,6 +32,7 @@ import org.apache.impala.thrift.TAlterTableType;
import org.apache.impala.util.KuduUtil;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -74,7 +75,9 @@ public class AlterTableAddColsStmt extends AlterTableStmt {
// are all valid and unique, and that none of the columns conflict with
// partition columns.
Set<String> colNames = new HashSet<>();
- for (ColumnDef c: columnDefs_) {
+ Iterator<ColumnDef> iterator = columnDefs_.iterator();
+ while (iterator.hasNext()){
+ ColumnDef c = iterator.next();
c.analyze(analyzer);
String colName = c.getColName().toLowerCase();
if (existingPartitionKeys.contains(colName)) {
@@ -83,9 +86,15 @@ public class AlterTableAddColsStmt extends AlterTableStmt {
}
Column col = t.getColumn(colName);
- if (col != null && !ifNotExists_) {
- throw new AnalysisException("Column already exists: " + colName);
- } else if (!colNames.add(colName)) {
+ if (col != null) {
+ if (!ifNotExists_) {
+ throw new AnalysisException("Column already exists: " + colName);
+ }
+ // remove the existing column
+ iterator.remove();
+ continue;
+ }
+ if (!colNames.add(colName)) {
throw new AnalysisException("Duplicate column name: " + colName);
}
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index dcf11da26..bc8f1af1a 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -1035,10 +1035,16 @@ public class CatalogOpExecutor {
}
switch (params.getAlter_type()) {
case ADD_COLUMNS:
- TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
- boolean added = alterTableAddCols(tbl, addColParams.getColumns(),
- addColParams.isIf_not_exists());
- reloadTableSchema = true;
+ boolean added = false;
+ // Columns could be ignored/cleared in AlterTableAddColsStmt,
+ // that may cause columns to be empty.
+ if (params.getAdd_cols_params() != null
+ && params.getAdd_cols_params().getColumnsSize() != 0) {
+ TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
+ added = alterTableAddCols(tbl, addColParams.getColumns(),
+ addColParams.isIf_not_exists());
+ reloadTableSchema = true;
+ }
if (added) {
responseSummaryMsg = "New column(s) have been added to the table.";
} else {
@@ -1267,9 +1273,14 @@ public class CatalogOpExecutor {
Preconditions.checkState(tbl.isWriteLockedByCurrentThread());
switch (params.getAlter_type()) {
case ADD_COLUMNS:
- TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
- KuduCatalogOpExecutor.addColumn(tbl, addColParams.getColumns());
- addSummary(response, "Column(s) have been added.");
+ if (params.getAdd_cols_params() != null
+ && params.getAdd_cols_params().getColumnsSize() != 0) {
+ TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
+ KuduCatalogOpExecutor.addColumn(tbl, addColParams.getColumns());
+ addSummary(response, "Column(s) have been added.");
+ } else {
+ addSummary(response, "No new column(s) have been added to the table.");
+ }
break;
case REPLACE_COLUMNS:
TAlterTableReplaceColsParams replaceColParams = params.getReplace_cols_params();
@@ -1334,9 +1345,14 @@ public class CatalogOpExecutor {
org.apache.iceberg.Transaction iceTxn = IcebergUtil.getIcebergTransaction(tbl);
switch (params.getAlter_type()) {
case ADD_COLUMNS:
- TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
- IcebergCatalogOpExecutor.addColumns(iceTxn, addColParams.getColumns());
- addSummary(response, "Column(s) have been added.");
+ if (params.getAdd_cols_params() != null
+ && params.getAdd_cols_params().getColumnsSize() != 0) {
+ TAlterTableAddColsParams addColParams = params.getAdd_cols_params();
+ IcebergCatalogOpExecutor.addColumns(iceTxn, addColParams.getColumns());
+ addSummary(response, "Column(s) have been added.");
+ } else {
+ addSummary(response, "No new column(s) have been added to the table.");
+ }
break;
case DROP_COLUMN:
TAlterTableDropColParams dropColParams = params.getDrop_col_params();
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
index 5f90441d7..ed6d8d775 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
@@ -729,6 +729,11 @@ public class AnalyzeKuduDDLTest extends FrontendTestBase {
AnalyzesOk("alter table functional_kudu.testtbl add columns (a int encoding rle)");
AnalyzesOk("alter table functional_kudu.testtbl add columns (a int compression lz4)");
AnalyzesOk("alter table functional_kudu.testtbl add columns (a int block_size 10)");
+ // Test 'if not exists'
+ AnalyzesOk("alter table functional_kudu.testtbl add if not exists columns " +
+ "(name string null)");
+ AnalyzesOk("alter table functional_kudu.testtbl add if not exists columns " +
+ "(a string null, b int, c string null)");
// REPLACE columns is not supported for Kudu tables
AnalysisError("alter table functional_kudu.testtbl replace columns (a int null)",
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
index b653199d0..5d20f8660 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
@@ -436,3 +436,60 @@ DESCRIBE FORMATTED iceberg_upgrade_v2_merge_mode;
---- TYPES
string, string, string
====
+---- QUERY
+# Add a column that already exists and a new column that does not exist with
+# "if not exists" clause.
+ALTER TABLE ice_alter_cols ADD IF NOT EXISTS COLUMNS (a bigint, d bigint)
+---- RESULTS
+'Column(s) have been added.'
+---- TYPES
+string
+====
+---- QUERY
+describe ice_alter_cols;
+---- RESULTS
+'d','decimal(22,3)','','true'
+'a','bigint','','true'
+---- TYPES
+STRING,STRING,STRING,STRING
+====
+---- QUERY
+# Add column for the same name, but of a different type
+ALTER TABLE ice_alter_cols ADD IF NOT EXISTS COLUMNS (a string, d string)
+---- RESULTS
+'No new column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
+# Add a column that already exists and a new column that does not exist with
+# "if not exists" clause.
+ALTER TABLE ice_alter_cols ADD IF NOT EXISTS COLUMNS (a bigint, b bigint)
+---- RESULTS
+'Column(s) have been added.'
+---- TYPES
+string
+====
+---- QUERY
+DESCRIBE ice_alter_cols
+---- RESULTS
+'d','decimal(22,3)','','true'
+'a','bigint','','true'
+'b','bigint','','true'
+---- TYPES
+STRING,STRING,STRING,STRING
+====
+---- QUERY
+ALTER TABLE ice_alter_cols ADD IF NOT EXISTS COLUMNS (c bigint, c string)
+---- CATCH
+AnalysisException: Duplicate column name: c
+====
+---- QUERY
+ALTER TABLE ice_alter_cols DROP COLUMN a;
+ALTER TABLE ice_alter_cols DROP COLUMN b;
+DESCRIBE ice_alter_cols;
+---- RESULTS
+'d','decimal(22,3)','','true'
+---- TYPES
+STRING,STRING,STRING,STRING
+====
\ No newline at end of file
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
index 63e5c6a45..0ed93f69f 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
@@ -347,6 +347,98 @@ alter table tbl_to_alter add columns (invalid_col int not null)
A new non-null column must have a default value
====
---- QUERY
+# Add a column that already exists and a new column that does not exist.
+alter table tbl_to_alter add columns (new_col4 string, new_col5 int)
+---- CATCH
+AnalysisException: Column already exists: new_col4
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT
+====
+---- QUERY
+# Add a column that already exists and a new column that does not exist with
+# "if not exists" clause.
+alter table tbl_to_alter add if not exists columns (new_col4 int, new_col5 int)
+---- RESULTS
+'Column(s) have been added.'
+---- TYPES
+string
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4, NEW_COL5
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT,INT
+====
+---- QUERY
+# Add a column that already exists and a new column that does not exist with
+# "if not exists" clause.
+alter table tbl_to_alter add if not exists columns (new_col4 string, new_col6 int)
+---- RESULTS
+'Column(s) have been added.'
+---- TYPES
+string
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4, NEW_COL5, NEW_COL6
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT,INT,INT
+====
+---- QUERY
+# All new columns are existing columns.
+alter table tbl_to_alter add if not exists columns (new_col3 string, new_col4 int);
+---- RESULTS
+'No new column(s) have been added to the table.'
+---- TYPES
+string
+====
+---- QUERY
+# Test for duplicated columns.
+alter table tbl_to_alter add if not exists columns (new_col7 string, new_col7 int);
+---- CATCH
+AnalysisException: Duplicate column name: new_col7
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4, NEW_COL5, NEW_COL6
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT,INT,INT
+====
+---- QUERY
+# Recover status after add 'new_col5' column
+alter table tbl_to_alter drop column new_col5
+---- RESULTS
+'Column has been dropped.'
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4, NEW_COL6
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT,INT
+====
+---- QUERY
+# Recover status after add 'new_col6' column
+alter table tbl_to_alter drop column new_col6
+---- RESULTS
+'Column has been dropped.'
+====
+---- QUERY
+describe tbl_to_alter;
+---- LABELS
+ID, NAME, VALI, NEW_COL1, NEW_COL2, NEW_COL3, NEW_COL4
+---- TYPES
+INT,STRING,BIGINT,INT,BIGINT,STRING,INT
+====
+---- QUERY
# Add a column with name reserved by Kudu engine
alter table tbl_to_alter add columns (auto_incrementing_id bigint)
---- CATCH
[impala] 03/03: IMPALA-11920: [DOCS] Cleanup and update spill examples
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 1321b5ce54b4c1d70715ffde9c898612ac9f3ed8
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Wed Feb 15 15:18:16 2023 -0800
IMPALA-11920: [DOCS] Cleanup and update spill examples
Updates documentation to include examples with service identifier. Also
fixes inconsistent use of ASCII quotes for example text, highlighting
code and variable names, and normalizes descriptions between
S3/HDFS/Ozone. Removes "priority" from remote descriptions as it is
optional and does nothing.
Change-Id: I624a607bda33ab47100e1540ff1d66c8d19a7329
Reviewed-on: http://gerrit.cloudera.org:8080/19504
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Michael Smith <mi...@cloudera.com>
---
docs/topics/impala_disk_space.xml | 153 ++++++++++++++++++++++++--------------
1 file changed, 96 insertions(+), 57 deletions(-)
diff --git a/docs/topics/impala_disk_space.xml b/docs/topics/impala_disk_space.xml
index 4440f7763..97eb9b37d 100644
--- a/docs/topics/impala_disk_space.xml
+++ b/docs/topics/impala_disk_space.xml
@@ -168,8 +168,7 @@ under the License.
sort, join, aggregation, or analytic function operations The files are
removed when the operation finishes. You can specify locations of the
intermediate files by starting the <cmdname>impalad</cmdname> daemon with
- the
- <codeph>‑‑scratch_dirs="<varname>path_to_directory</varname>"</codeph>
+ the <codeph>--scratch_dirs="<varname>path_to_directory</varname>"</codeph>
configuration option. By default, intermediate files are stored in the
directory <filepath>/tmp/impala-scratch</filepath>.<p
id="order_by_scratch_dir">
@@ -279,7 +278,7 @@ under the License.
<section>
<title>Priority Based Scratch Directory Selection</title>
<p>The location of the intermediate files are configured by starting the impalad daemon with
- the flag ‑‑scratch_dirs="path_to_directory". Currently this startup flag uses the configured
+ the flag <codeph>--scratch_dirs="path_to_directory"</codeph>. Currently this startup flag uses the configured
scratch directories in a round robin fashion. Automatic selection of scratch directories in
a round robin fashion may not always be ideal in every situation since these directories
could come from different classes of storage system volumes having different performance
@@ -290,28 +289,25 @@ under the License.
priorities of the directories and if you provide the same priority for multiple directories
then the directories will be selected in a round robin fashion.</p>
<p>The valid formats for specifying the priority directories are as shown here:
- <codeblock>
- <dir-path>:<limit>:<priority>
- <dir-path>::<priority>
+ <codeblock><varname>dir-path</varname>:<varname>limit</varname>:<varname>priority</varname>
+<varname>dir-path</varname>::<varname>priority</varname>
</codeblock></p>
<p>Example:</p>
<p>
- <codeblock>
- /dir1:200GB:0
- /dir1::0
+ <codeblock>/dir1:200GB:0
+/dir1::0
</codeblock>
</p>
<p>The following formats use the default priority:
- <codeblock>
- /dir1
- /dir1:200GB
- /dir1:200GB:
+ <codeblock>/dir1
+/dir1:200GB
+/dir1:200GB:
</codeblock>
</p>
<p>In the example below, dir1 will be used as a spill victim until it is full and then dir2, dir3,
and dir4 will be used in a round robin fashion.</p>
<p>
- <codeblock>‑‑scratch_dirs="/dir1:200GB:0, /dir2:1024GB:1, /dir3:1024GB:1, /dir4:1024GB:1"
+ <codeblock>--scratch_dirs="/dir1:200GB:0, /dir2:1024GB:1, /dir3:1024GB:1, /dir4:1024GB:1"
</codeblock>
</p>
</section>
@@ -349,8 +345,8 @@ under the License.
large sorts, joins, aggregations, or analytic function operations. If your workload results
in large volumes of intermediate data being written, it is recommended to configure the
heavy spilling queries to use a remote storage location rather than the local one. The
- advantage of using remote storage for scratch space is that it is elastic and can handle any
- amount of spilling.</p>
+ advantage of using remote storage for scratch space is that it is elastic and can handle
+ any amount of spilling.</p>
<p><b>Before you begin</b></p>
<p>Identify the URL for an S3 bucket to which you want your new Impala to write the temporary
data. If you use the S3 bucket that is associated with the environment, navigate to the S3
@@ -358,8 +354,10 @@ under the License.
your environment to use the external S3 bucket with the correct read/write permissions.</p>
<p><b>Configuring the Start-up Option in Impala daemon</b></p>
<p>You can use the Impalad start option scratch_dirs to specify the locations of the
- intermediate files. The format of the option is <codeph>scratch_dirs= remote_dir, local_buffer_dir(,
- local_dir…).</codeph></p>
+ intermediate files. The format of the option is:</p>
+ <codeblock>--scratch_dirs="<varname>remote_dir</varname>, <varname>local_buffer_dir</varname> (,<varname>local_dir</varname>…)"</codeblock>
+ <p>where <varname>local_buffer_dir</varname> and <varname>local_dir</varname> conform to the
+ earlier descriptions for scratch directories.</p>
<p>With the option specified above:</p>
<ul>
<li>You can specify only one remote directory. When you configure a remote directory, you
@@ -368,18 +366,20 @@ under the License.
first local directory would be used as the local buffer directory.</li>
<li>If you configure both remote and local directories, the remote directory is only used
when the local directories are fully utilized.</li>
- <li>The size of a remote intermediate file could affect the query performance, and the value
- can be set by <codeph>>remote_tmp_file_size</codeph> in the start-up option. The default
- size of a remote intermediate file is 16MB while the maximum is 256MB.</li>
+ <li>The size of a remote intermediate file could affect the query performance, and the
+ value can be set by <codeph>--remote_tmp_file_size=<varname>size</varname></codeph> in
+ the start-up option. The default size of a remote intermediate file is 16MB while the
+ maximum is 512MB.</li>
</ul>
<p><b>Examples</b></p>
<ul>
- <li>A remote scratch dir with one local buffer dir, file size 64MB.
- <codeblock>‑‑scratch_dirs="s3a://remote_dir, /local_buffer_dir" ‑‑remote_tmp_file_size=64M</codeblock></li>
- <li>A remote scratch dir with one local buffer dir, and one local dir.
- <codeblock>‑‑scratch_dirs="s3a://remote_dir, /local_buffer_dir, /local_dir"</codeblock></li>
- <li>A remote scratch dir with one local buffer dir, and multiple local dirs.
- <codeblock>‑‑scratch_dirs="s3a://remote_dir, /local_buffer_dir, /local_dir_1, /local_dir_2"</codeblock></li>
+ <li>A remote scratch dir with a local buffer dir, file size 64MB.
+ <codeblock>--scratch_dirs=s3a://remote_dir,/local_buffer_dir --remote_tmp_file_size=64M</codeblock></li>
+ <li>A remote scratch dir with a local buffer dir limited to 256MB, and one local dir
+ limited to 10GB.
+ <codeblock>--scratch_dirs=s3a://remote_dir,/local_buffer_dir:256M,/local_dir:10G</codeblock></li>
+ <li>A remote scratch dir with a local buffer dir, and multiple prioritized local dirs.
+ <codeblock>--scratch_dirs=s3a://remote_dir,/local_buffer_dir,/local_dir_1:5G:1,/local_dir_2:5G:2</codeblock></li>
</ul>
</section>
<section>
@@ -388,52 +388,55 @@ under the License.
large sorts, joins, aggregations, or analytic function operations. If your workload results
in large volumes of intermediate data being written, it is recommended to configure the
heavy spilling queries to use a remote storage location rather than the local one. The
- advantage of using remote storage for scratch space is that it is elastic and can handle any
- amount of spilling.</p>
+ advantage of using remote storage for scratch space is that it is elastic and can handle
+ any amount of spilling.</p>
<p><b>Before you begin</b></p>
<ul>
<li>Identify the HDFS scratch directory where you want your new Impala to write the
temporary data.</li>
- <li>Identify the port number of the HDFS scratch directory.</li>
+ <li>Identify the IP address, host name, or service identifier of HDFS.</li>
+ <li>Identify the port number of the HDFS NameNode (if not-default).</li>
<li>Configure Impala to write temporary data to disk during query processing.</li>
</ul>
<p><b>Configuring the Start-up Option in Impala daemon</b></p>
- <p>You can use the Impalad start option “scratch_dirs” to specify the locations of the
- intermediate files.</p>
+ <p>You can use the Impalad start option <codeph>scratch_dirs</codeph> to specify the
+ locations of the intermediate files.</p>
<p>Use the following format for this start up option:</p>
- <codeblock>‑‑scratch_dirs=”hdfs://ip_address:port_num(:max_bytes)(:priority), /local_buffer_dir” ‑‑remote_tmp_file_size=xM</codeblock>
+ <codeblock>--scratch_dirs="hdfs://<varname>authority</varname>/<varname>path</varname>(:<varname>max_bytes</varname>), <varname>local_buffer_dir</varname> (,<varname>local_dir</varname>…)"</codeblock>
<ul>
- <li>Where <codeph>“hdfs://ip_address:port_num/path(:max_bytes)(:priority)”</codeph> is the remote
- directory.</li>
- <li><codeph>port_num</codeph> is required for the HDFS scratch directory.</li>
- <li><codeph>max_bytes</codeph> and <codeph>priority</codeph> are optional.</li>
+ <li>Where <codeph>hdfs://<varname>authority</varname>/<varname>path</varname></codeph> is
+ the remote directory.</li>
+ <li><varname>authority</varname> may include <codeph>ip_address</codeph> or
+ <codeph>hostname</codeph> and <codeph>port</codeph>, or <codeph>service_id</codeph>.</li>
+ <li><varname>max_bytes</varname> is optional.</li>
</ul>
<p>Using the above format:</p>
<ul>
- <li>You can specify only one remote directory.</li>
- <li>When you configure a remote directory, you must specify a local buffer directory as the
- buffer. However you can use multiple local directories with the remote directory. If you
- specify multiple local directories, the first local directory would be used as the local
- buffer directory.</li>
+ <li>You can specify only one remote directory. When you configure a remote directory, you
+ must specify a local buffer directory as the buffer. However you can use multiple local
+ directories with the remote directory. If you specify multiple local directories, the
+ first local directory would be used as the local buffer directory.</li>
<li>If you configure both remote and local directories, the remote directory is only used
when the local directories are fully utilized.</li>
- <li>The size of a remote intermediate file could affect the query performance, and the value
- can be set by “remote_tmp_file_size” in the start-up option. The default size of a remote
- intermediate file is 16MB while the maximum is 512MB.</li>
+ <li>The size of a remote intermediate file could affect the query performance, and the
+ value can be set by <codeph>--remote_tmp_file_size=<varname>size</varname></codeph> in
+ the start-up option. The default size of a remote intermediate file is 16MB while the
+ maximum is 512MB.</li>
</ul>
<p><b>Examples</b></p>
<ul>
- <li>A hdfs scratch dir with one local buffer dir, file size 64MB. The space of hdfs scratch
+ <li>A HDFS scratch dir with one local buffer dir, file size 64MB. The space of HDFS scratch
dir is limited to 300G.
- <codeblock>‑‑scratch_dirs="hdfs://ip_address:port_num/path:300G, /local_buffer_dir" ‑‑remote_tmp_file_size=64M</codeblock></li>
- <li>A hdfs scratch dir with one local buffer dir, and one local dir. The space of hdfs
- scratch dir is limited to 300G.
- <codeblock>‑‑scratch_dirs="hdfs://ip_address:port_num/path:300G, /local_buffer_dir, /local_dir"</codeblock></li>
- <li>A hdfs scratch dir with one local buffer dir, and multiple local dirs. The space of hdfs
- scratch dir is unlimited.
- <codeblock>‑‑scratch_dirs="hdfs://ip_address:port_num/path, /local_buffer_dir, /local_dir_1, /local_dir_2"</codeblock></li>
+ <codeblock>--scratch_dirs=hdfs://10.0.0.49:20500/tmp:300G,/local_buffer_dir --remote_tmp_file_size=64M</codeblock></li>
+ <li>A HDFS scratch dir with one local buffer dir limited to 512MB, and one local dir
+ limited to 10GB. The space of HDFS scratch dir is limited to 300G. The HDFS NameNode uses
+ its default port (8020).
+ <codeblock>--scratch_dirs=hdfs://hdfsnn/tmp:300G,/local_buffer_dir:512M,/local_dir:10G</codeblock></li>
+ <li>A HDFS scratch dir with one local buffer dir, and multiple prioritized local dirs. The
+ space of HDFS scratch dir is unlimited. The HDFS service identifier is <codeph>hdfs1</codeph>.
+ <codeblock>--scratch_dirs=hdfs://hdfs1/tmp,/local_buffer_dir,/local_dir_1:5G:1,/local_dir_2:5G:2</codeblock></li>
</ul>
- <p>Even though max_bytes is optional it is highly recommended to configure for spilling to
+ <p>Even though max_bytes is optional, it is highly recommended to configure for spilling to
HDFS because the HDFS cluster space is limited.</p>
</section>
<section>
@@ -442,12 +445,48 @@ under the License.
<ul>
<li>Identify the Ozone scratch directory where you want your new Impala to write the
temporary data.</li>
- <li>Identify the port number of the Ozone scratch directory.</li>
+ <li>Identify the IP address, host name, or service identifier of Ozone.</li>
+ <li>Identify the port number of the Ozone Manager (if not-default).</li>
</ul>
<p><b>Configuring the Start-up Option in Impala daemon</b></p>
- <p>You can use the Impalad start option “scratch_dirs” to specify the locations of the
+ <p>You can use the Impalad start option <codeph>scratch_dirs</codeph> to specify the locations of the
intermediate files.</p>
- <codeblock>‑‑scratch_dirs=”ofs://ip_address:port_num(:max_bytes)(:priority), /local_buffer_dir” ‑‑remote_tmp_file_size=xM</codeblock>
+ <codeblock>--scratch_dirs="ofs://<varname>authority</varname>/<varname>path</varname>(:<varname>max_bytes</varname>), <varname>local_buffer_dir</varname> (,<varname>local_dir</varname>…)"</codeblock>
+ <ul>
+ <li>Where <codeph>ofs://<varname>authority</varname>/<varname>path</varname></codeph> is
+ the remote directory.</li>
+ <li><codeph>authority</codeph> may include <codeph>ip_address</codeph> or
+ <codeph>hostname</codeph> and <codeph>port</codeph>, or <codeph>service_id</codeph>.</li>
+ <li><codeph>max_bytes</codeph> is optional.</li>
+ </ul>
+ <p>Using the above format:</p>
+ <ul>
+ <li>You can specify only one remote directory. When you configure a remote directory, you
+ must specify a local buffer directory as the buffer. However you can use multiple local
+ directories with the remote directory. If you specify multiple local directories, the
+ first local directory would be used as the local buffer directory.</li>
+ <li>If you configure both remote and local directories, the remote directory is only used
+ when the local directories are fully utilized.</li>
+ <li>The size of a remote intermediate file could affect the query performance, and the
+ value can be set by <codeph>--remote_tmp_file_size=<varname>size</varname></codeph> in
+ the start-up option. The default size of a remote intermediate file is 16MB while the
+ maximum is 512MB.</li>
+ </ul>
+ <p><b>Examples</b></p>
+ <ul>
+ <li>An Ozone scratch dir with one local buffer dir, file size 64MB. The space of Ozone
+ scratch dir is limited to 300G.
+ <codeblock>--scratch_dirs=ofs://10.0.0.49:29000/tmp:300G,/local_buffer_dir --remote_tmp_file_size=64M</codeblock></li>
+ <li>An Ozone scratch dir with one local buffer dir limited to 512MB, and one local dir
+ limited to 10GB. The space of Ozone scratch dir is limited to 300G. The Ozone Manager
+ uses its default port (9862).
+ <codeblock>--scratch_dirs=ofs://ozonemgr/tmp:300G,/local_buffer_dir:512M,/local_dir:10G</codeblock></li>
+ <li>An Ozone scratch dir with one local buffer dir, and multiple prioritized local dirs. The
+ space of Ozone scratch dir is unlimited. The Ozone service identifier is <codeph>ozone1</codeph>.
+ <codeblock>--scratch_dirs=ofs://ozone1/tmp,/local_buffer_dir,/local_dir_1:5G:1,/local_dir_2:5G:2</codeblock></li>
+ </ul>
+ <p>Even though max_bytes is optional, it is highly recommended to configure for spilling to
+ Ozone because the Ozone cluster space is limited.</p>
</section>
</conbody>
[impala] 02/03: IMPALA-11920: Support spill to HDFS address by service ID
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 99d676f8fb71304838c8fde70d3dd220f8f1f52a
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Mon Feb 13 11:09:56 2023 -0800
IMPALA-11920: Support spill to HDFS address by service ID
Allows addressing HDFS (and Ozone) filesystems in `scratch_dirs` by a
service identifier that doesn't include a port number. Examples
- "hdfs://hdfs1/:10G" uses the root directory of HDFS with a 10G limit
- "ofs://ozone1/tmp::" uses /tmp in Ozone with default limit/priority
Updates `scratch_dirs` parsing to allow whitespace after each specifier,
as in "hdfs://hdfs1/ , /tmp". This is unambiguous and avoids failures
for simple mistakes.
Testing:
- new backend test cases run with HDFS and Ozone
- manually tested that Impala starts with
--impalad_args=--scratch_dirs=ofs://localhost/tmp,/tmp
creates impala-scratch in both locations
Change-Id: Ie069cba211df85fe90d174900b20a26fcc1f7167
Reviewed-on: http://gerrit.cloudera.org:8080/19496
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Michael Smith <mi...@cloudera.com>
---
be/src/runtime/tmp-file-mgr-test.cc | 72 ++++++++++++++++++++++---------------
be/src/runtime/tmp-file-mgr.cc | 22 ++++++++----
2 files changed, 59 insertions(+), 35 deletions(-)
diff --git a/be/src/runtime/tmp-file-mgr-test.cc b/be/src/runtime/tmp-file-mgr-test.cc
index f29f12adb..69b80c975 100644
--- a/be/src/runtime/tmp-file-mgr-test.cc
+++ b/be/src/runtime/tmp-file-mgr-test.cc
@@ -1056,7 +1056,7 @@ TEST_F(TmpFileMgrTest, TestDirectoryLimitParsing) {
// directories.
auto& dirs2 = GetTmpDirs(
CreateTmpFileMgr("/tmp/tmp-file-mgr-test1:foo,/tmp/tmp-file-mgr-test2:?,"
- "/tmp/tmp-file-mgr-test3:1.2.3.4,/tmp/tmp-file-mgr-test4: ,"
+ "/tmp/tmp-file-mgr-test3:1.2.3.4,/tmp/tmp-file-mgr-test4:a,"
"/tmp/tmp-file-mgr-test5:5pb,/tmp/tmp-file-mgr-test6:10%,"
"/tmp/tmp-file-mgr-test1:100"));
EXPECT_EQ(1, dirs2.size());
@@ -1111,10 +1111,10 @@ TEST_F(TmpFileMgrTest, TestDirectoryLimitParsingRemotePath) {
EXPECT_EQ(full_hdfs_path, dirs3->path());
EXPECT_EQ(1024, dirs3->bytes_limit());
- // Multiple local paths with one remote path.
+ // Multiple local paths with one remote path. Trims spaces.
auto tmp_mgr_4 = CreateTmpFileMgr(hdfs_path
- + ",/tmp/local-buffer-dir1,"
- "/tmp/local-buffer-dir2,/tmp/local-buffer-dir3");
+ + " , /tmp/local-buffer-dir1, "
+ "/tmp/local-buffer-dir2 ,/tmp/local-buffer-dir3");
auto& dirs4_local = GetTmpDirs(tmp_mgr_4);
auto& dirs4_remote = GetTmpRemoteDir(tmp_mgr_4);
EXPECT_NE(nullptr, dirs4_remote);
@@ -1123,8 +1123,8 @@ TEST_F(TmpFileMgrTest, TestDirectoryLimitParsingRemotePath) {
EXPECT_EQ("/tmp/local-buffer-dir2/impala-scratch", dirs4_local[0]->path());
EXPECT_EQ("/tmp/local-buffer-dir3/impala-scratch", dirs4_local[1]->path());
- // Fails the parsing due to no port number for the HDFS path.
- auto tmp_mgr_5 = CreateTmpFileMgr("hdfs://localhost/tmp,/tmp/local-buffer-dir");
+ // Fails to parse HDFS URI due to no path element.
+ auto tmp_mgr_5 = CreateTmpFileMgr("hdfs://localhost,/tmp/local-buffer-dir");
auto& dirs5_local = GetTmpDirs(tmp_mgr_5);
auto& dirs5_remote = GetTmpRemoteDir(tmp_mgr_5);
EXPECT_EQ(1, dirs5_local.size());
@@ -1136,33 +1136,27 @@ TEST_F(TmpFileMgrTest, TestDirectoryLimitParsingRemotePath) {
// Parse successfully, but the parsed HDFS path is unable to connect.
// These cases would fail the initialization of TmpFileMgr.
- auto& dirs7 = GetTmpRemoteDir(
- CreateTmpFileMgr("hdfs://localhost:1/tmp::1,/tmp/local-buffer-dir", false));
- EXPECT_EQ(nullptr, dirs7);
-
- auto& dirs8 = GetTmpRemoteDir(
- CreateTmpFileMgr("hdfs://localhost:/tmp::,/tmp/local-buffer-dir", false));
- EXPECT_EQ(nullptr, dirs8);
-
- auto& dirs9 = GetTmpRemoteDir(
- CreateTmpFileMgr("hdfs://localhost/tmp::1,/tmp/local-buffer-dir", false));
- EXPECT_EQ(nullptr, dirs9);
-
- auto& dirs10 = GetTmpRemoteDir(
- CreateTmpFileMgr("hdfs://localhost/tmp:1,/tmp/local-buffer-dir", false));
- EXPECT_EQ(nullptr, dirs10);
+ for (const string& unreachable_path : {
+ "hdfs://localhost:1/tmp::1", "hdfs://localhost:/tmp::", "hdfs://localhost/tmp::1",
+ "hdfs://localhost/tmp:1", "hdfs://localhost/tmp", "hdfs://localhost/:1",
+ "hdfs://localhost:1 ", "hdfs://localhost/ "
+ }) {
+ auto& dirs = GetTmpRemoteDir(
+ CreateTmpFileMgr(unreachable_path + ",/tmp/local-buffer-dir", false));
+ EXPECT_EQ(nullptr, dirs) << unreachable_path;
+ }
// Multiple remote paths, should support only one.
- auto& dirs11 = GetTmpRemoteDir(CreateTmpFileMgr(Substitute(
+ auto& dirs6 = GetTmpRemoteDir(CreateTmpFileMgr(Substitute(
"$0,hdfs://localhost:20501/tmp,/tmp/local-buffer-dir", hdfs_path)));
- EXPECT_NE(nullptr, dirs11);
- EXPECT_EQ(full_hdfs_path, dirs11->path());
+ EXPECT_NE(nullptr, dirs6);
+ EXPECT_EQ(full_hdfs_path, dirs6->path());
// The order of the buffer and the remote dir should not affect the result.
- auto& dirs12 = GetTmpRemoteDir(CreateTmpFileMgr(Substitute(
+ auto& dirs7 = GetTmpRemoteDir(CreateTmpFileMgr(Substitute(
"/tmp/local-buffer-dir,$0,hdfs://localhost:20501/tmp", hdfs_path)));
- EXPECT_NE(nullptr, dirs12);
- EXPECT_EQ(full_hdfs_path, dirs12->path());
+ EXPECT_NE(nullptr, dirs7);
+ EXPECT_EQ(full_hdfs_path, dirs7->path());
}
// Successful cases for parsing S3 paths.
@@ -1385,7 +1379,7 @@ TEST_F(TmpFileMgrTest, TestDirectoryPriorityParsing) {
// directories.
auto& dirs2 = GetTmpDirs(
CreateTmpFileMgr("/tmp/tmp-file-mgr-test1::foo,/tmp/tmp-file-mgr-test2::?,"
- "/tmp/tmp-file-mgr-test3::1.2.3.4,/tmp/tmp-file-mgr-test4:: ,"
+ "/tmp/tmp-file-mgr-test3::1.2.3.4,/tmp/tmp-file-mgr-test4::a,"
"/tmp/tmp-file-mgr-test5::p0,/tmp/tmp-file-mgr-test6::10%,"
"/tmp/tmp-file-mgr-test1:100:-1"));
EXPECT_EQ(1, dirs2.size());
@@ -2236,4 +2230,26 @@ TEST_F(TmpFileMgrTest, TestBatchReadingSetMaxBytes) {
}
}
+TEST_F(TmpFileMgrTest, TestHdfsScratchParsing) {
+ struct parsed { string path; int64_t bytes_limit; int priority; };
+ constexpr int64_t dbytes = numeric_limits<int64_t>::max();
+ constexpr int dpriority = numeric_limits<int>::max();
+ for (auto& valid : map<string, parsed>{
+ { "hdfs://10.0.0.1:8020", parsed{"hdfs://10.0.0.1:8020", dbytes, dpriority} },
+ { "hdfs://10.0.0.1:8020:1024:", parsed{"hdfs://10.0.0.1:8020", 1024, dpriority} },
+ { "hdfs://10.0.0.1/", parsed{"hdfs://10.0.0.1", dbytes, dpriority} },
+ { "hdfs://10.0.0.1/:1k", parsed{"hdfs://10.0.0.1", 1024, dpriority} },
+ { "hdfs://localhost/tmp::", parsed{"hdfs://localhost/tmp", dbytes, dpriority} },
+ { "hdfs://localhost/tmp/:10k:1", parsed{"hdfs://localhost/tmp", 10240, 1} },
+ { "ofs://ozone1/tmp:10k:1", parsed{"ofs://ozone1/tmp", 10240, 1} },
+ { "ofs://ozone1/tmp::1", parsed{"ofs://ozone1/tmp", dbytes, 1} },
+ }) {
+ TmpDirHdfs fs(valid.first);
+ EXPECT_OK(fs.Parse());
+ EXPECT_EQ(path(valid.second.path) / "impala-scratch", fs.path());
+ EXPECT_EQ(valid.second.bytes_limit, fs.bytes_limit());
+ EXPECT_EQ(valid.second.priority, fs.priority());
+ }
+}
+
} // namespace impala
diff --git a/be/src/runtime/tmp-file-mgr.cc b/be/src/runtime/tmp-file-mgr.cc
index f8af71a30..ea871700c 100644
--- a/be/src/runtime/tmp-file-mgr.cc
+++ b/be/src/runtime/tmp-file-mgr.cc
@@ -287,7 +287,7 @@ Status TmpFileMgr::InitCustom(const vector<string>& tmp_dir_specifiers,
// warning - we don't want to abort process startup because of misconfigured scratch,
// since queries will generally still be runnable.
for (const string& tmp_dir_spec : tmp_dir_specifiers) {
- string tmp_dir_spec_trimmed(boost::algorithm::trim_left_copy(tmp_dir_spec));
+ string tmp_dir_spec_trimmed(boost::algorithm::trim_copy(tmp_dir_spec));
std::unique_ptr<TmpDir> tmp_dir;
if (IsHdfsPath(tmp_dir_spec_trimmed.c_str(), false)
@@ -811,15 +811,23 @@ Status TmpDirS3::VerifyAndCreate(MetricGroup* metrics, vector<bool>* is_tmp_dir_
}
Status TmpDirHdfs::ParsePathTokens(vector<string>& toks) {
- // We enforce the HDFS scratch path to have the port number, and the format after split
- // by colon is {scheme, path, port_num, [bytes_limit, [priority]]}. Coalesce the URI.
+ // HDFS scratch path can include an optional port number; URI without path and port
+ // number is ambiguous so in that case we error. Format after split by colon is
+ // {scheme, path, port_num?, [bytes_limit, [priority]]}. Coalesce the URI from tokens.
split(toks, raw_path_, is_any_of(":"), token_compress_off);
- if (toks.size() < 3) {
+ // Only called on paths starting with `hdfs://` or `ofs://`.
+ DCHECK(toks.size() >= 2);
+ if (toks[1].rfind("/") > 1) {
+ // Contains a slash after the scheme, so port number was omitted.
+ toks[0] = Substitute("$0:$1", toks[0], toks[1]);
+ toks.erase(toks.begin()+1);
+ } else if (toks.size() < 3) {
return Status(
- Substitute("The scratch path should have the port number: '$0'", raw_path_));
+ Substitute("The scratch URI must have a path or port number: '$0'", raw_path_));
+ } else {
+ toks[0] = Substitute("$0:$1:$2", toks[0], toks[1], toks[2]);
+ toks.erase(toks.begin()+1, toks.begin()+3);
}
- toks[0] = Substitute("$0:$1:$2", toks[0], toks[1], toks[2]);
- toks.erase(toks.begin()+1, toks.begin()+3);
return Status::OK();
}