You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2024/02/23 19:19:36 UTC

[Impala-ASF-CR] IMPALA-12840: Deflake test reuse partitions transactional

Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21064


Change subject: IMPALA-12840: Deflake test_reuse_partitions_transactional
......................................................................

IMPALA-12840: Deflake test_reuse_partitions_transactional

TestReusePartitions::test_reuse_partitions_transactional calls
/catalog_object path of CatalogD's WebUI and decodes the JSON response.
The "json_string" field from the response text often contains unicode
control character that will throw error such as following upon decoding:

ValueError: Invalid control character at: line 1 column 1850 (char 1849)

This control character exist after the legal JSON string. This patch
attempt to deflake the test by passing strict=False argument to
json.loads() function.
https://docs.python.org/2/library/json.html#encoders-and-decoders

This patch also print the response / JSON object if assertion fail.

Testing:
- Loop and pass the test for hundred times.
- Manually hack the test to proof that "json_string" is successfully
  parsed even when unicode control character exist.

Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
---
M tests/metadata/test_reuse_partitions.py
1 file changed, 7 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Gerrit-Change-Number: 21064
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21064 )

Change subject: IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21064/2/fe/src/main/java/org/apache/impala/catalog/Table.java
File fe/src/main/java/org/apache/impala/catalog/Table.java:

http://gerrit.cloudera.org:8080/#/c/21064/2/fe/src/main/java/org/apache/impala/catalog/Table.java@709
PS2, Line 709:    * Variant of toThrift() with lower detail. Intended to get a human readable output
> nit: This is also returned in a user-facing API. Debugging to me implies de
Done


http://gerrit.cloudera.org:8080/#/c/21064/2/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:

http://gerrit.cloudera.org:8080/#/c/21064/2/fe/src/main/java/org/apache/impala/service/JniCatalog.java@404
PS2, Line 404:    * slimmer than the one obtained from getCatalogObject() because binary data fields
> typo: slimmer
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Gerrit-Change-Number: 21064
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Feb 2024 19:35:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21064 )

Change subject: IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject
......................................................................

IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

TestReusePartitions::test_reuse_partitions_transactional calls
/catalog_object path of CatalogD's WebUI and decodes the JSON response.
The "json_string" field from the response text often contains Unicode
control characters that come from serialized binary data from
THdfsFileDesc objects. That causes JSON decoding to fail with an error
like this:

ValueError: Invalid control character at: line 1 column 1850 (char 1849)

This patch attempts to deflake the test by tuning the return value of
getJsonCatalogObject() that excludes THdfsFileDesc, by lowering the
detail level from ThriftObjectType.FULL to
ThriftObjectType.DESCRIPTOR_ONLY.

test_reuse_partitions_transactional is tweaked a bit to print the
response / JSON object if an assertion fails.

Testing:
- Loop and pass the test for hundred times.

Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Reviewed-on: http://gerrit.cloudera.org:8080/21064
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
M tests/metadata/test_reuse_partitions.py
6 files changed, 54 insertions(+), 7 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Gerrit-Change-Number: 21064
Gerrit-PatchSet: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21064 )

Change subject: IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject
......................................................................


Patch Set 4: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21064/3/fe/src/main/java/org/apache/impala/catalog/Catalog.java
File fe/src/main/java/org/apache/impala/catalog/Catalog.java:

http://gerrit.cloudera.org:8080/#/c/21064/3/fe/src/main/java/org/apache/impala/catalog/Catalog.java@551
PS3, Line 551:    *                        catalog object that omit some field.
> typo: omit
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Gerrit-Change-Number: 21064
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Feb 2024 19:55:18 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21064 )

Change subject: IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Gerrit-Change-Number: 21064
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Feb 2024 00:08:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject
......................................................................

IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

TestReusePartitions::test_reuse_partitions_transactional calls
/catalog_object path of CatalogD's WebUI and decodes the JSON response.
The "json_string" field from the response text often contains Unicode
control characters that come from serialized binary data from
THdfsFileDesc objects. That causes JSON decoding to fail with an error
like this:

ValueError: Invalid control character at: line 1 column 1850 (char 1849)

This patch attempts to deflake the test by tuning the return value of
getJsonCatalogObject() that excludes THdfsFileDesc, by lowering the
detail level from ThriftObjectType.FULL to
ThriftObjectType.DESCRIPTOR_ONLY.

test_reuse_partitions_transactional is tweaked a bit to print the
response / JSON object if an assertion fails.

Testing:
- Loop and pass the test for hundred times.

Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
M tests/metadata/test_reuse_partitions.py
6 files changed, 52 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Gerrit-Change-Number: 21064
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Michael Smith, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject
......................................................................

IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

TestReusePartitions::test_reuse_partitions_transactional calls
/catalog_object path of CatalogD's WebUI and decodes the JSON response.
The "json_string" field from the response text often contains Unicode
control characters that come from serialized binary data from
THdfsFileDesc objects. That causes JSON decoding to fail with an error
like this:

ValueError: Invalid control character at: line 1 column 1850 (char 1849)

This patch attempts to deflake the test by tuning the return value of
getJsonCatalogObject() that excludes THdfsFileDesc, by lowering the
detail level from ThriftObjectType.FULL to
ThriftObjectType.DESCRIPTOR_ONLY.

test_reuse_partitions_transactional is tweaked a bit to print the
response / JSON object if an assertion fails.

Testing:
- Loop and pass the test for hundred times.

Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
M tests/metadata/test_reuse_partitions.py
6 files changed, 54 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Gerrit-Change-Number: 21064
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21064 )

Change subject: IMPALA-12840: Exclude THdfsFileDesc in getJsonCatalogObject
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6840bf1267d1d99d321c0a6b4a0cab49543182
Gerrit-Change-Number: 21064
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Feb 2024 00:08:21 +0000
Gerrit-HasComments: No