-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix: Ensure consistent test behavior with NonDex by resolving nondeterministic field order #5765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…rministic ordering issue with use of sorting. - Ensured deterministic test data setup to prevent intermittent failures caused by NonDex.
fix: Ensure consistent test behavior with NonDex by resolving nondeterministic behaviour
|
Arrays.sort(f, new Comparator<Field>() { | ||
@Override | ||
public int compare(Field a, Field b) { | ||
return a.getName().compareTo(b.getName()); | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This almost seems like a test requirement rather than doing it in production code. Can't we pass the fields in sorted order within the test?
I would consider running a flaky test Jenkins job for determining whether the fix will resolve the issue. Please consider using this Jenkins job - https://ci.hive.apache.org/blue/organizations/jenkins/hive-flaky-check/activity @ayushtkn @zabetak @deniskuzZ I dont seem to have access to retrigger / run this flaky test job as well. Is there a way I can get access to it. |
Hey @SourabhBadhya , I just gave you access to trigger jobs in hiveci. Currently, permissions are granted per request. For more info please see https://issues.apache.org/jira/browse/HIVE-28641 |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
re-triggered the build |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
What changes were proposed in this pull request?
This pull request addresses non-deterministic behavior in the
TestLazyBinaryColumnarSerDe#testLazyBinaryColumnarSerDeWithEmptyBinary
test and other related tests. The root cause of the intermittent failures was identified inObjectInspectorUtils.java
, where the methodgetDeclaredFields()
was used. ThegetDeclaredFields()
method returns fields in an unordered manner, which can cause inconsistent results across different JVM versions and vendors. This PR resolves the issue by sorting the fields retrieved bygetDeclaredFields()
to ensure consistent behavior across JVMs.Why are the changes needed?
The
getDeclaredFields()
method does not guarantee a specific order in the returned fields, which can vary depending on the JVM version and vendor. This non-deterministic behavior caused test cases, such asTestLazyBinaryColumnarSerDe#testLazyBinaryColumnarSerDeWithEmptyBinary
, to fail intermittently. By sorting the fields, we ensure deterministic behavior, thus resolving the root cause of these test failures.Does this PR introduce any user-facing change?
No, this PR does not introduce any user-facing changes. The changes are internal, aimed at stabilizing the behavior of the code and ensuring that tests pass consistently across environments.
Is the change a dependency upgrade?
No, this PR does not involve any dependency upgrades. It focuses solely on addressing the non-deterministic behavior in the current codebase.
How was this patch tested?
The patch was tested by running the affected test cases, particularly
TestLazyBinaryColumnarSerDe#testLazyBinaryColumnarSerDeWithEmptyBinary
, using theNonDex
tool. This tool helps detect and debug issues with under-determined Java APIs. The sorting of fields was confirmed to prevent non-deterministic behavior, and the tests were verified to pass consistently.To replicate the testing steps:
mvn install
.hive/serde
directory.cd hive/serde mvn edu.illinois:nondex-maven-plugin:2.1.7:nondex -Dtest=TestLazyBinaryColumnarSerDe#testLazyBinaryColumnarSerDeWithEmptyBinary -DnondexMode=FULL -DnondexRuns=10