You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Smith (Code Review)" <ge...@cloudera.org> on 2022/08/09 22:50:12 UTC

[Impala-ASF-CR] IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests

Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18828


Change subject: IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests
......................................................................

IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests

Use lazy initialization for TMP_DIR_PREFIX_LIST so not all tests have to
create a BackendConfig. Some tests do using FeSupport.loadLibrary, and
the new tests for FileSystemUtil required initializing
BackendConfig.INSTANCE.

Failure to initialize BackendConfig meant that test success would depend
on their order. If FileSystemUtilTest or another test that initialized
it were the first to use FileSystemUtil, then everything would pass. If
not, the static declaration in FileSystemUtil would fail, causing the
class to fail to be loaded. Later tests would fail with NoClassDefFound
errors.

Adds initialization to FileMetadataLoaderTest as it has explicit hidden
file tests. Any tests that test hidden file functionality must ensure
BackendConfig is loaded so TMP_DIR_PREFIX_LIST is correctly initialized.
Without that, hidden file filtering will not work correctly.

Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
---
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java
2 files changed, 27 insertions(+), 10 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11133/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 00:06:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Initialize BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Initialize BackendConfig for tests
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8427/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 10 Aug 2022 16:55:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests
......................................................................


Patch Set 1: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/8423/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 10 Aug 2022 04:30:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Initialize BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Initialize BackendConfig for tests
......................................................................


Patch Set 2: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/8427/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 10 Aug 2022 21:46:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11126/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 09 Aug 2022 23:10:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Initialize BackendConfig for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Joe McDonnell, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11469: (Addendum) Initialize BackendConfig for tests
......................................................................

IMPALA-11469: (Addendum) Initialize BackendConfig for tests

Initialize BackendConfig in FileSystemUtil for unit tests. Checking
ignored dirs can happen in lots of tests, so rather than updating every
test class to initialize this we provide a fallback. Tests that need a
specific BackendConfig can override it by calling `create` again.

If BackendConfig is initialized this way, we log a warning as this
should not happen when running impalad. The implementation of
IMPALA-11469 already depends on static initialization time (as defined
https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4)
to ensure BackendConfig is initialized via JniCatalog or JniFrontend.

Failure to initialize BackendConfig meant that test success would depend
on their order. If FileSystemUtilTest or another test that initialized
it were the first to use FileSystemUtil, then everything would pass. If
not, the static declaration in FileSystemUtil would fail, causing the
class to fail to be loaded. Later tests would fail with NoClassDefFound
errors.

Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
---
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
2 files changed, 9 insertions(+), 9 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Joe McDonnell, Impala Public Jenkins, Xiang Yang, 

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

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

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

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................

IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Moves ignored_dir_prefix_list parsing to BackendConfig creation to avoid
static blocks. The default constructor for TBackendGflags - used in some
tests - leaves the property uninitialized, so guards against nulls.

Adds FrontendTestBase base class to relevant tests to ensure
BackendConfig is set.

Failure to initialize BackendConfig meant that test success would depend
on their order. If FileSystemUtilTest or another test that initialized
it were the first to use FileSystemUtil, then everything would pass. If
not, the static declaration in FileSystemUtil would fail, causing the
class to fail to be loaded. Later tests would fail with NoClassDefFound
errors.

Testing:
- Ran frontend tests individually.

Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
---
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java
M fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
4 files changed, 30 insertions(+), 31 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>

[Impala-ASF-CR] IMPALA-11469: (Addendum) Initialize BackendConfig for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Initialize BackendConfig for tests
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18828/2/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:

http://gerrit.cloudera.org:8080/#/c/18828/2/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@858
PS2, Line 858:     if (BackendConfig.INSTANCE == null) {
This seems hacky. We should switch to all frontend tests to extend FrontendTestBase, and use that to initialize BackendConfig.


http://gerrit.cloudera.org:8080/#/c/18828/2/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@866
PS2, Line 866:     String s = BackendConfig.INSTANCE.getIgnoredDirPrefixList();
This could be refactored to remove the static block by moving list construction to BackendConfig and providing an accessor there. I'm not sure if that brings any performance implications.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 10 Aug 2022 17:12:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18828/3/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:

http://gerrit.cloudera.org:8080/#/c/18828/3/fe/src/main/java/org/apache/impala/service/BackendConfig.java@80
PS3, Line 80:   public static void create(TBackendGflags cfg, boolean initialize) {
make 'INSTANCE' public and mutable is terrible,  especially the assignment is not synchronized. maybe  we can add 'synchronized' to decorate the create() method and add 'volatile' to decorate the  'INSTANCE' property like Double-Checked Lock to alleviate the problem?
or we can just remove the 'INSTANCE' property, every call should cache a instance by itself.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 06:12:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Joe McDonnell, Impala Public Jenkins, Xiang Yang, 

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

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

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

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................

IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Moves ignored_dir_prefix_list parsing to BackendConfig creation to avoid
static blocks. The default constructor for TBackendGflags - used in some
tests - leaves the property uninitialized, so guards against nulls.

Testing:
- Ran frontend tests individually.

Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
---
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
2 files changed, 26 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/18828/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18828/3/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:

http://gerrit.cloudera.org:8080/#/c/18828/3/fe/src/main/java/org/apache/impala/service/BackendConfig.java@80
PS3, Line 80:   public static void create(TBackendGflags cfg, boolean initialize) {
> make 'INSTANCE' public and mutable is terrible,  especially the assignment 
I believe INSTANCE is only initialized once during startup in normal execution. I don't think those changes are necessary for correctness, but I don't see any harm in adding them for clarity. However that seems unrelated to this patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 16:00:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18828/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:

http://gerrit.cloudera.org:8080/#/c/18828/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@873
PS1, Line 873:           TMP_DIR_PREFIX_LIST.add(prefix);
> Good point.
What I'd really like to do is have code that initializes BackendConfig when we start running tests. I haven't figured out how to do that yet, but I'll dig at it a little more.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 10 Aug 2022 15:53:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 05:02:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests
......................................................................


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8423/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 09 Aug 2022 23:12:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Tue, 16 Aug 2022 22:24:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8451/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Tue, 16 Aug 2022 17:35:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11162/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Tue, 16 Aug 2022 16:41:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Initialize BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Initialize BackendConfig for tests
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11130/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 10 Aug 2022 17:02:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8433/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 11 Aug 2022 00:03:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Joe McDonnell, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................

IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Moves ignored_dir_prefix_list parsing to BackendConfig creation to avoid
static blocks. The default constructor for TBackendGflags - used in some
tests - leaves the property uninitialized, so guards against nulls.

Adds FrontendTestBase base class to relevant tests to ensure
BackendConfig is set.

Failure to initialize BackendConfig meant that test success would depend
on their order. If FileSystemUtilTest or another test that initialized
it were the first to use FileSystemUtil, then everything would pass. If
not, the static declaration in FileSystemUtil would fail, causing the
class to fail to be loaded. Later tests would fail with NoClassDefFound
errors.

Testing:
- Ran frontend tests individually.

Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
---
M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java
M fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
4 files changed, 31 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/18828/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18828/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:

http://gerrit.cloudera.org:8080/#/c/18828/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@873
PS1, Line 873:           TMP_DIR_PREFIX_LIST.add(prefix);
This is not thread-safe. FileSystemUtil is used in multiple threads concurrently. We could have a corrupt list here. E.g. at the middle of the for-loop, another thread initializes 'TMP_DIR_PREFIX_LIST' to a new empty list.

I tend to inititialize BackendConfig in the tests to reduce the complexity here. Or we can inititialize BackendConfig in the current static clause.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 10 Aug 2022 00:26:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Lazy initialize tmp dir list for tests
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18828/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:

http://gerrit.cloudera.org:8080/#/c/18828/1/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java@873
PS1, Line 873:           TMP_DIR_PREFIX_LIST.add(prefix);
> This is not thread-safe. FileSystemUtil is used in multiple threads concurr
Good point.

Finding all the tests that might hit this code seems challenging, since it's used for general path traversal. So I guess we have to initialize BackendConfig in the static clause.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 10 Aug 2022 15:34:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18828/3/fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java
File fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java:

http://gerrit.cloudera.org:8080/#/c/18828/3/fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java@43
PS3, Line 43: FrontendTestBase
> It's not clear to me how this fix the NullPointerException. I might have mi
I finally figure it out by printing the stacktrace in BackendConfig.create(). It's quite subtle:

 at org.apache.impala.service.BackendConfig.create(BackendConfig.java:59)
 at org.apache.impala.service.BackendConfig.create(BackendConfig.java:53)
 at org.apache.impala.service.JniFrontend.<init>(JniFrontend.java:137)
 at org.apache.impala.service.FeSupport.NativeFeInit(Native Method)
 at org.apache.impala.service.FeSupport.loadLibrary(FeSupport.java:494)
 at org.apache.impala.service.FeSupport.LookupSymbol(FeSupport.java:213)
 at org.apache.impala.service.FeSupport.LookupSymbol(FeSupport.java:222)
 at org.apache.impala.catalog.Function.lookupSymbol(Function.java:475)
 at org.apache.impala.catalog.ScalarFunction.createBuiltin(ScalarFunction.java:184)
 at org.apache.impala.catalog.ScalarFunction.createBuiltinOperator(ScalarFunction.java:172)
 at org.apache.impala.catalog.ScalarFunction.createBuiltinOperator(ScalarFunction.java:167)
 at org.apache.impala.analysis.ArithmeticExpr.initBuiltins(ArithmeticExpr.java:105)
 at org.apache.impala.catalog.BuiltinsDb.initBuiltins(BuiltinsDb.java:105)
 at org.apache.impala.catalog.BuiltinsDb.<init>(BuiltinsDb.java:94)
 at org.apache.impala.catalog.BuiltinsDb.getInstance(BuiltinsDb.java:79)
 at org.apache.impala.catalog.ImpaladCatalog.<init>(ImpaladCatalog.java:113)
 at org.apache.impala.testutil.ImpaladTestCatalog.<init>(ImpaladTestCatalog.java:58)
 at org.apache.impala.testutil.ImpaladTestCatalog.<init>(ImpaladTestCatalog.java:51)
 at org.apache.impala.common.FrontendFixture.<init>(FrontendFixture.java:101)
 at org.apache.impala.common.FrontendFixture.<clinit>(FrontendFixture.java:94)
 at org.apache.impala.common.AbstractFrontendTest.<clinit>(AbstractFrontendTest.java:42)
 at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
 at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
 at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 at java.lang.reflect.Method.invoke(Method.java:498)
 at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
 at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
 at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
 at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
 at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
 at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
 at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
 at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:272)
 at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:236)
 at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
 at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:386)
 at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:323)
 at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:143)

The output is in ${IMPALA_HOME}/logs/fe_tests/org.apache.impala.catalog.FileMetadataLoaderTest-output.txt



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Thu, 18 Aug 2022 06:47:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 5:

Debatable whether this is useful. Static blocks in Java don't seem to be as troublesome as they are in C++.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Thu, 18 Aug 2022 19:10:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/11186/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Thu, 18 Aug 2022 19:31:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11469: (Addendum) Refactor BackendConfig for tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has abandoned this change. ( http://gerrit.cloudera.org:8080/18828 )

Change subject: IMPALA-11469: (Addendum) Refactor BackendConfig for tests
......................................................................


Abandoned

No strong reason to refactor this. I prefer avoiding statics where possible, but I think that's mostly aesthetic in this case.
-- 
To view, visit http://gerrit.cloudera.org:8080/18828
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I802fcbf70808f2127197f720a7247c3d85389d6f
Gerrit-Change-Number: 18828
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>