[impala] branch master updated: IMPALA-9702: Cleanup unique_database directories

[jo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new bfdc5bf  IMPALA-9702: Cleanup unique_database directories
bfdc5bf is described below

commit bfdc5bf6af2703127d4ff5611ed049f11b2cb004
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Sun May 31 14:56:01 2020 -0700

    IMPALA-9702: Cleanup unique_database directories
    
    If there are external tables in a database, drop database cascade
    won't remove the external table locations. If those locations
    are inside the database, then the database directory does not
    get removed. Some tests that use unique_database fail when
    running for the second time (or with a data snapshot) due to the
    preexisting files.
    
    This adds code to remove the database directory for
    unique_database. It also adds some debugging statements that
    list the files at the beginning of bin/run-all-tests.sh and
    again at the end.
    
    Testing:
     - Ran a core job and verified that the unique database
       directories are being removed
     - Ran TestMixedPartitions::test_incompatible_avro_partition_in_non_avro_table()
       multiple times and it passes when it previously failed.
    
    Change-Id: I0530c028e5e7c241dfc054f04c78e2a045c2d035
    Reviewed-on: http://gerrit.cloudera.org:8080/16015
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 bin/run-all-tests.sh |  9 +++++++++
 tests/conftest.py    | 23 ++++++++++++++++-------
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/bin/run-all-tests.sh b/bin/run-all-tests.sh
index 7e61068..77ce7c4 100755
--- a/bin/run-all-tests.sh
+++ b/bin/run-all-tests.sh
@@ -171,6 +171,9 @@ for i in $(seq 1 $NUM_TEST_ITERATIONS)
 do
   TEST_RET_CODE=0
 
+  # Store a list of the files at the beginning of each iteration.
+  hdfs dfs -ls -R /test-warehouse > ${IMPALA_LOGS_DIR}/file-list-begin-${i}.log 2>&1
+
   start_impala_cluster
 
   if [[ "$BE_TEST" == true ]]; then
@@ -276,6 +279,12 @@ do
   # succeed.
   # ${IMPALA_HOME}/tests/run-process-failure-tests.sh
 
+  # Store a list of the files at the end of each iteration. This can be compared
+  # to the file-list-begin*.log from the beginning of the iteration to see if files
+  # are not being cleaned up. This is most useful on the first iteration, when
+  # the list of files is from dataload.
+  hdfs dfs -ls -R /test-warehouse > ${IMPALA_LOGS_DIR}/file-list-end-${i}.log 2>&1
+
   # Finally, kill the spawned timeout process and its child sleep process.
   # There may not be a sleep process, so ignore failure.
   pkill -P $TIMEOUT_PID || true
diff --git a/tests/conftest.py b/tests/conftest.py
index b544c5e..f8c98f6 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -34,7 +34,7 @@ from tests.common.environ import build_flavor_timeout
 from common.test_result_verifier import QueryTestResult
 from tests.common.patterns import is_valid_impala_identifier
 from tests.comparison.db_connection import ImpalaConnection
-from tests.util.filesystem_utils import FILESYSTEM, ISILON_WEBHDFS_PORT
+from tests.util.filesystem_utils import FILESYSTEM, ISILON_WEBHDFS_PORT, WAREHOUSE
 
 LOG = logging.getLogger('test_configuration')
 LOG_FORMAT = "-- %(asctime)s %(levelname)-8s %(threadName)s: %(message)s"
@@ -343,22 +343,31 @@ def unique_database(request, testid_checksum):
                        ' test function name or any prefixes for long length or invalid '
                        'characters.'.format(db_name))
 
+  def cleanup_database(db_name, must_exist):
+    request.instance.execute_query_expect_success(request.instance.client,
+        'DROP DATABASE {0} `{1}` CASCADE'.format(
+            "" if must_exist else "IF EXISTS", db_name),
+        {'sync_ddl': sync_ddl})
+    # The database directory may not be removed if there are external tables in the
+    # database when it is dropped. The external locations are not removed by cascade.
+    # These preexisting files/directories can cause errors when tests run repeatedly or
+    # use a data snapshot (see IMPALA-9702), so this forces cleanup of the database
+    # directory.
+    db_location = "{0}/{1}.db".format(WAREHOUSE, db_name).lstrip('/')
+    request.instance.filesystem_client.delete_file_dir(db_location, recursive=True)
+
   def cleanup():
     # Make sure we don't try to drop the current session database
     request.instance.execute_query_expect_success(request.instance.client, "use default")
     for db_name in db_names:
-      request.instance.execute_query_expect_success(
-          request.instance.client, 'DROP DATABASE `{0}` CASCADE'.format(db_name),
-          {'sync_ddl': sync_ddl})
+      cleanup_database(db_name, True)
       LOG.info('Dropped database "{0}" for test ID "{1}"'.format(
           db_name, str(request.node.nodeid)))
 
   request.addfinalizer(cleanup)
 
   for db_name in db_names:
-    request.instance.execute_query_expect_success(
-        request.instance.client, 'DROP DATABASE IF EXISTS `{0}` CASCADE'.format(db_name),
-        {'sync_ddl': sync_ddl})
+    cleanup_database(db_name, False)
     request.instance.execute_query_expect_success(
         request.instance.client, 'CREATE DATABASE `{0}`'.format(db_name),
         {'sync_ddl': sync_ddl})