[SPARK-31014][CORE][3.0] InMemoryStore: remove key from parentToChildrenMap when removing key from CountingRemoveIfForEach #27825
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…p when removing key from CountingRemoveIfForEach ### What changes were proposed in this pull request? This patch addresses missed spot on SPARK-30964 (apache#27716) - SPARK-30964 added secondary index which defines the relationship between parent - children and able to operate all children for given parent faster. While SPARK-30964 handled the addition and deletion of secondary index in InstanceList properly, it missed to add code to handle deletion of secondary index in CountingRemoveIfForEach, resulting to the leak of indices. This patch adds the deletion of secondary index in CountingRemoveIfForEach. ### Why are the changes needed? Described above. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? N/A, as relevant field and class are marked as private, and it cannot be checked in higher level. I'm not sure we want to adjust scope to add a test. Closes apache#27765 from HeartSaVioR/SPARK-31014. Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan.opensource@gmail.com> Signed-off-by: Gengliang Wang <gengliang.wang@databricks.com>
|
cc. @gengliangwang |
gengliangwang
approved these changes
Mar 6, 2020
|
Test build #119443 has finished for PR 27825 at commit
|
Weird failure. Will check the recent branch-3.0. |
|
OK we should be very careful while cherry-picking commit to I'll raise a PR to fix it soon. |
|
#27826 to fix build failure. |
|
Thanks, @HeartSaVioR . The PR is merged. |
|
Retest this please. |
|
Test build #119446 has finished for PR 27825 at commit
|
|
retest this, please |
|
Test build #119454 has finished for PR 27825 at commit
|
|
retest this, please |
|
Test build #119462 has finished for PR 27825 at commit
|
dongjoon-hyun
approved these changes
Mar 7, 2020
There was a problem hiding this comment.
+1, LGTM. Thank you, @HeartSaVioR and @gengliangwang .
Merged to 3.0.
dongjoon-hyun
pushed a commit
that referenced
this pull request
Mar 7, 2020
…renMap when removing key from CountingRemoveIfForEach ### What changes were proposed in this pull request? This patch addresses missed spot on SPARK-30964 (#27716) - SPARK-30964 added secondary index which defines the relationship between parent - children and able to operate all children for given parent faster. While SPARK-30964 handled the addition and deletion of secondary index in InstanceList properly, it missed to add code to handle deletion of secondary index in CountingRemoveIfForEach, resulting to the leak of indices. This patch adds the deletion of secondary index in CountingRemoveIfForEach. ### Why are the changes needed? Described above. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? N/A, as relevant field and class are marked as private, and it cannot be checked in higher level. I'm not sure we want to adjust scope to add a test. Closes #27825 from HeartSaVioR/SPARK-31014-branch-3.0. Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan.opensource@gmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
|
Thanks for reviewing and merging! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This patch addresses missed spot on SPARK-30964 (#27716) - SPARK-30964 added secondary index which defines the relationship between parent - children and able to operate all children for given parent faster.
While SPARK-30964 handled the addition and deletion of secondary index in InstanceList properly, it missed to add code to handle deletion of secondary index in CountingRemoveIfForEach, resulting to the leak of indices. This patch adds the deletion of secondary index in CountingRemoveIfForEach.
Why are the changes needed?
Described above.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
N/A, as relevant field and class are marked as private, and it cannot be checked in higher level. I'm not sure we want to adjust scope to add a test.