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.

Sign up for GitHub

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

Jump to bottom

Bulk Export Query Splitter #3922

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open

Bulk Export Query Splitter #3922

wants to merge 20 commits into from

Conversation

ab295382
Copy link
Member

@ab295382 ab295382 commented Dec 13, 2024 •
edited
Loading

Make sure you have checked all steps below.

Issue

Tests

  • My PR adds the following tests OR does not need testing for this extremely good reason:
    • MyUnitTest

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it, or I have linked to a
    separate issue for that below.
  • If I have added or removed any dependencies from the project, I have updated the NOTICES file.

@ab295382 ab295382 linked an issue Dec 13, 2024 that may be closed by this pull request
Bulk Export Query Splitter #3818
Open
@ab295382 ab295382 added the enhancement New feature or request label Dec 13, 2024
@ab295382 ab295382 self-assigned this Dec 13, 2024
@ab295382 ab295382 marked this pull request as ready for review December 17, 2024 15:16
patchwork01
patchwork01 reviewed Dec 18, 2024
View reviewed changes
...port-core/src/main/java/sleeper/bulkexport/core/recordretrieval/BulkExportQuerySplitter.java Outdated Show resolved Hide resolved
...port-core/src/main/java/sleeper/bulkexport/core/recordretrieval/BulkExportQuerySplitter.java Outdated Show resolved Hide resolved
...port-core/src/main/java/sleeper/bulkexport/core/recordretrieval/BulkExportQuerySplitter.java Outdated Show resolved Hide resolved
...port-core/src/main/java/sleeper/bulkexport/core/recordretrieval/BulkExportQuerySplitter.java
private List<Partition> leafPartitions;
private PartitionTree partitionTree;
private Map<String, List<String>> partitionToFiles;
private Instant nextInitialiseTime;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like the initialisation behaviour is untested. Have a look at QueryExecutorTest.ReinitialiseOnTimeout.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tests added

...-core/src/test/java/sleeper/bulkexport/core/recordretrieval/BulkExportQuerySplitterTest.java Outdated Show resolved Hide resolved
...core/src/main/java/sleeper/bulkexport/core/recordretrieval/LeafPartitionRecordRetriever.java Outdated Show resolved Hide resolved
...port-core/src/main/java/sleeper/bulkexport/core/recordretrieval/BulkExportQuerySplitter.java Outdated Show resolved Hide resolved
@ab295382 ab295382 added this to the 0.28.0 milestone Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patchwork01 patchwork01 Awaiting requested review from patchwork01

@gaffer01 gaffer01 Awaiting requested review from gaffer01

@rtjd6554 rtjd6554 Awaiting requested review from rtjd6554

Assignees

@gaffer01 gaffer01

@patchwork01 patchwork01

@rtjd6554 rtjd6554

Labels
enhancement New feature or request
Projects
None yet
Milestone
0.28.0
Development

Successfully merging this pull request may close these issues.

Bulk Export Query Splitter
4 participants
@ab295382 @patchwork01 @gaffer01 @rtjd6554