Skip to content
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

Restore classic Join and Merge handlers, make index-based handlers optional #1275

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

florianesser
Copy link
Member

Functionally identical to this hale-core PR

Restore the classes JoinHandler and PropertiesMergeHandler as the default handlers for the transformation functions Join, Groovy Join, Merge and Groovy Merge.

The previous default handlers IndexJoinHandler and IndexMergeHandler have demonstrated negative impact on the performance of some transformations. Until this is better understood and fixed, these handlers are now optional and can be activated via the following Java properties and environment variables:

  • Join/Groovy Join: hale.functions.use_index_join_handler / HALE_FUNCTIONS_USE_INDEX_JOIN_HANDLER
  • Merge/Groovy Merge: hale.functions.use_index_merge_handler / HALE_FUNCTIONS_USE_INDEX_MERGE_HANDLER

ING-4464

@@ -21,7 +21,10 @@
import java.util.Collections;
import java.util.List;

import de.fhg.igd.slf4jplus.ALogger;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build error hints at package import may be missing in manifest?

@florianesser florianesser force-pushed the perf/join-merge-handler branch from 25c894f to 3687be2 Compare November 7, 2024 11:42
Copy link

github-actions bot commented Nov 7, 2024

hale studio builds for this pull request:

Build triggered for commit 8e61992.
Artifacts are retained for 14 days.

@florianesser
Copy link
Member Author

/autosquash

Restore the classes `JoinHandler` and `PropertiesMergeHandler` as the default
handlers for the transformation functions Join, Groovy Join, Merge and Groovy
Merge.

The previous default handlers `IndexJoinHandler` and `IndexMergeHandler` have
demonstrated negative impact on the performance of some transformations. Until
this is better understood and fixed, these handlers are now optional and can be
activated via the following Java properties and environment variables:

- Join/Groovy Join: `hale.functions.use_index_join_handler` / `HALE_FUNCTIONS_USE_INDEX_JOIN_HANDLER`
- Merge/Groovy Merge: `hale.functions.use_index_merge_handler` / `HALE_FUNCTIONS_USE_INDEX_MERGE_HANDLER`

ING-4464
@florianesser florianesser force-pushed the perf/join-merge-handler branch from 3687be2 to a688c7c Compare November 11, 2024 20:09
@florianesser
Copy link
Member Author

Merging w/o challenge as performance improvement was tested as part of the hale-core PR and verification of the functionality of the new switches is not essential.

@florianesser florianesser merged commit 5092689 into master Nov 11, 2024
7 of 8 checks passed
@florianesser florianesser deleted the perf/join-merge-handler branch November 11, 2024 20:48
@florianesser florianesser added the challenged For PRs to indicate that the implementation has been challenged label Dec 6, 2024
@florianesser
Copy link
Member Author

Tested function of HALE_FUNCTIONS_USE_INDEX_JOIN_HANDLER environment variable with test project where correct results depend on the classic join handler.

Copy link

we-release bot commented Dec 6, 2024

🎉 This PR is included in version 5.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@we-release we-release bot added the released label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
challenged For PRs to indicate that the implementation has been challenged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants