Skip to content

fixed testSerDe flaky test#1

Open
yesh385 wants to merge 1 commit into
masterfrom
fix-flaky
Open

fixed testSerDe flaky test#1
yesh385 wants to merge 1 commit into
masterfrom
fix-flaky

Conversation

@yesh385

@yesh385 yesh385 commented Sep 3, 2023

Copy link
Copy Markdown
Owner

This PR fixes a flaky test called TestLazyBinaryColumnarSerDe.testSerDe which can be found here.

  1. How was this test identified as flaky?

This test was identifies as flaky by using an open-source research tool named NonDex which is responsible for finding and diagnosing non-deterministic runtime exceptions in Java programs.

  1. What does this test do?

This test is responsible for testing serialization and deserialization of data using a specific implementation called LazyBinaryColumnarSerDe. This test is used to ensure that the serialization and deserialization of an object of type OuterStruct works correctly.

  1. Why this test is flaky?

This test is flaky as there is an order mismatch between the different fields of the object inspector oi in the serialization process.

The error occurs here:

BytesRefArrayWritable braw = (BytesRefArrayWritable) serde.serialize(outerStruct, oi);

Specifically, there is a mismatch between the field f and the field object inspector foi during serialization causing the field to be serialized using the incorrect field object inspector which results in a java.lang.ClassCastException.

LazyBinarySerDe.serialize(serializeStream, f, foi, true, warnedOnceNullMapKey);

  1. How I fixed this test?

This PR fixes this error by sorting the fields of the object inspector oi based on the slot property.

The field list is sorted here:

In the above code, we are using the Arrays.sort method to sort the array f using a custom comparator. In this comparator, we try to get a reference to the private field named slot in the class Field using reflection. We then set it to be accessible and retrieve the value of the field slot for the current object and return it as an integer. In the event that, there is an error with the reflection or if the slot field does not exist, we catch it and print the stack trace. In case of an exception, we return 0 to ensure that we provide a fallback value that allows the program to continue executing.

You can run the following command to run the test using NonDex tool:

mvn edu.illinois:nondex-maven-plugin:2.1.1:nondex -pl serde -Dtest=org.apache.hadoop.hive.serde2.columnar.TestLazyBinaryColumnarSerDe#testSerDe

(Optional) You can also run the following command to run the test:

mvn -pl serde test -Dtest=org.apache.hadoop.hive.serde2.columnar.TestLazyBinaryColumnarSerDe#testSerDe

Test Environment:

java version "1.8.0_202"
Apache Maven 3.6.3

@yesh385 yesh385 changed the title fixed flaky test fixed testSerDe flaky test Oct 3, 2023
@Einsteinnnnn

Copy link
Copy Markdown

First of all, I think you need to explicitly state in the PR why the flaky is generated and the error code that reports the error. Just listing the nondex code used and the line of code in question doesn't make it easy for someone to know what went wrong and the details of the specific problem.
Secondly, you should explain in the PR why you made the changes you did, e.g. why you used a try-catch and why it should return 0 if there is an error.
The code you fixed may be valid, but a non-detailed PR description may cause your PR to be rejected.

@yesh385 yesh385 closed this Oct 15, 2023
@yesh385 yesh385 reopened this Oct 15, 2023
@yesh385

yesh385 commented Oct 15, 2023

Copy link
Copy Markdown
Owner Author

Updated the PR based on your feedback.

@zzjas

zzjas commented Oct 17, 2023

Copy link
Copy Markdown

You can proceed to open a real PR. Once you open a real PR, please mark this tentative PR as Opened in your tentative_pr.csv file and also raise a PR to IDoFT marking this as Opened. Thanks!

@rRajivramachandran

Copy link
Copy Markdown

The PR looks good, following are a few suggestions:

  1. Created this PR to fix a flaky test called TestLazyBinaryColumnarSerDe.testSerDe which can be found in the path:
    hive/serde/src/test/org/apache/hadoop/hive/serde2/columnar/TestLazyBinaryColumnarSerDe.java -> This PR fixes a flaky test
  2. Please add Java and maven version used by you
  3. Kindly squash all commits to 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants