Issue 1668 - Check files have the correct jobId assigned to them in compaction job state store update #1684

kr565370 · 2024-01-15T16:23:22Z

Rerunning functional system tests

Make sure you have checked all steps below.

Issue

My PR addresses the following issues and references them in the PR title. For example, "Issue 1234 - My Sleeper
PR"
- Resolves Compaction jobs can fail to process if one has been picked up by a task before it has been created #1668

Tests

My PR adds the following tests OR does not need testing for this extremely good reason:
- Updating existing tests
- Added new test for jobId not assigned to files in each FileReferenceStore implementation

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, removed, or updated any external dependencies used in the project, I have updated the
NOTICES file to reflect this.

…toreIT

…tore

…eck in state store update

...onfiguration/src/main/java/sleeper/configuration/properties/instance/CompactionProperty.java

...on-job-execution/src/main/java/sleeper/compaction/jobexecution/CompactSortedFilesRunner.java

java/statestore/src/test/java/sleeper/statestore/dynamodb/DynamoDBFileReferenceStoreIT.java

…eReferenceStoreTest

…ReportTest

…eTableIT

…edFilesRunnerLocalStackIT

…ector tests

…eReferenceStoreIT

…teStoreIT

…statestore-update # Conflicts: # java/compaction/compaction-job-execution/src/main/java/sleeper/compaction/jobexecution/CompactSortedFiles.java # java/core/src/test/java/sleeper/core/statestore/inmemory/InMemoryFileReferenceStoreTest.java # java/statestore/src/test/java/sleeper/statestore/dynamodb/DynamoDBFileReferenceStoreIT.java

…enceStoreIT

gaffer01 · 2024-01-17T13:45:16Z

...mpaction-job-execution/src/main/java/sleeper/compaction/jobexecution/CompactSortedFiles.java

@@ -147,7 +148,8 @@ private RecordsProcessed compactNoSplitting() throws IOException, IteratorExcept
        // Create writer
        LOGGER.debug("Creating writer for file {}", compactionJob.getOutputFile());
        Path outputPath = new Path(compactionJob.getOutputFile());
-        ParquetWriter<Record> writer = ParquetRecordWriterFactory.createParquetRecordWriter(outputPath, tableProperties, conf);
+        ParquetWriter<Record> writer = ParquetRecordWriterFactory.createParquetRecordWriter(
+                outputPath, tableProperties, conf, ParquetFileWriter.Mode.OVERWRITE);


Can you add a comment here saying why we've chosen to use OVERWRITE mode?

...system-test-drivers/src/test/java/sleeper/systemtest/drivers/util/WaitForJobsStatusTest.java

...est/system-test-drivers/src/main/java/sleeper/systemtest/drivers/util/WaitForJobsStatus.java

…es-have-job-ids-before-statestore-update

kr565370 added 4 commits January 15, 2024 16:01

Test new method for checking assigned jobId in DynamoDBFileReferenceS…

f188af0

…toreIT

Test new method for checking assigned jobId in S3FileReferenceStoreIT

20319b3

Test new method for checking assigned jobId in InMemoryFileReferenceS…

3fdcc65

…tore

Use new method to check jobId of input files in CompactSortedFiles

1edef6c

kr565370 changed the title ~~Issue 1668 - Check job files have the correct jobId assigned to them in compaction job state store updates~~ Issue 1668 - Check job files have the correct jobId assigned to them in compaction job state store update Jan 15, 2024

kr565370 changed the title ~~Issue 1668 - Check job files have the correct jobId assigned to them in compaction job state store update~~ Issue 1668 - Check files have the correct jobId assigned to them in compaction job state store update Jan 15, 2024

kr565370 added the pr-stacked-top A stacked pull request that we don't want to merge into its target until the target PR is merged label Jan 15, 2024

Fix failing test

a5ad22b

kr565370 assigned patchwork01 Jan 16, 2024

kr565370 linked an issue Jan 16, 2024 that may be closed by this pull request

Compaction jobs can fail to process if one has been picked up by a task before it has been created #1668

Closed

kr565370 added 7 commits January 16, 2024 11:37

Set write mode to OVERWRITE when performing standard compactions

4c44cc5

Only update job status of files for non-splitting jobs

23f6459

Group non-splitting actions in conditional block

009e592

Include cause exception in thrown StateStoreException

a7aa12d

Update job status of files for splitting jobs, include conditional ch…

e067b20

…eck in state store update

Convert CompactSortedFilesRunner to use builder

024cc9e

Introduce CompactionProperty for visibilty timeout for failed jobs

b18faf9

Base automatically changed from 1666-rename-fileinfo-to-filereference to develop January 17, 2024 09:50

patchwork01 reviewed Jan 17, 2024

View reviewed changes

kr565370 added 9 commits January 17, 2024 09:59

Update property name to be more specific about failed jobs

74945a9

Set clients explicitly in CompactSortedFilesRunner main method

b419680

Use new method signature for atomically updating files in InMemoryFil…

814f5a9

…eReferenceStoreTest

Use new method signature for atomically updating files in FilesStatus…

d6d02a9

…ReportTest

Use new method signature for atomically updating files in Reinitialis…

6d711dc

…eTableIT

Use new method signature for atomically updating files in CompactSort…

dbf403e

…edFilesRunnerLocalStackIT

Use new method signature for atomically updating files in GarbageColl…

c3fa44a

…ector tests

Use new method signature for atomically updating files in DynamoDBFil…

81eb5eb

…eReferenceStoreIT

Use new method signature for atomically updating files in DynamoDBSta…

4b13876

…teStoreIT

patchwork01 removed the pr-stacked-top A stacked pull request that we don't want to merge into its target until the target PR is merged label Jan 17, 2024

patchwork01 assigned kr565370 Jan 17, 2024

patchwork01 removed their assignment Jan 17, 2024

kr565370 added 6 commits January 17, 2024 11:10

Update properties templates

d5572bf

Fix test failure

e2357ff

Use new method signature for atomically updating files in S3FileRefer…

7333ffd

…enceStoreIT

Use new method signature for atomically updating files in S3StateStoreIT

a817ae1

Remove old method signature for atomically updating files

63a19a7

patchwork01 approved these changes Jan 17, 2024

View reviewed changes

patchwork01 assigned gaffer01 and unassigned kr565370 Jan 17, 2024

Add test for multiple job runs in WaitForJobStatusTest

7826821

gaffer01 reviewed Jan 17, 2024

View reviewed changes

kr565370 added 2 commits January 17, 2024 14:02

Revert to using boolean flag for in progress jobs

29d0e44

Refactor and revert changes

5a1c619

gaffer01 approved these changes Jan 17, 2024

View reviewed changes

gaffer01 removed their assignment Jan 17, 2024

patchwork01 reviewed Jan 17, 2024

View reviewed changes

...system-test-drivers/src/test/java/sleeper/systemtest/drivers/util/WaitForJobsStatusTest.java Outdated Show resolved Hide resolved

kr565370 added 2 commits January 17, 2024 14:40

Add comment to explain why we use ParquetFileWriter.Mode.OVERWRITE

98c873b

Fix incorrect firstInProgressStartTime in test

bb7727a

patchwork01 reviewed Jan 17, 2024

View reviewed changes

...est/system-test-drivers/src/main/java/sleeper/systemtest/drivers/util/WaitForJobsStatus.java Outdated Show resolved Hide resolved

patchwork01 added the pr-base-for-stacking Base for stacked pull requests (a dependency for others, where this PR's branch will be the base) label Jan 17, 2024

kr565370 and others added 5 commits January 17, 2024 15:26

Add test for 2 jobs with 2 runs

caa15f0

Revert changes and fix failing tests

34c1bd3

Handle case where an earlier run is finished in WaitForJobsStatus

f5662d7

Tweak WaitForJobsStatusTest

99c461f

Merge remote-tracking branch 'origin/develop' into 1668-check-job-fil…

0b1e825

…es-have-job-ids-before-statestore-update

patchwork01 approved these changes Jan 17, 2024

View reviewed changes

kr565370 merged commit 5561f87 into develop Jan 17, 2024
15 checks passed

kr565370 deleted the 1668-check-job-files-have-job-ids-before-statestore-update branch January 17, 2024 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 1668 - Check files have the correct jobId assigned to them in compaction job state store update #1684

Issue 1668 - Check files have the correct jobId assigned to them in compaction job state store update #1684

kr565370 commented Jan 15, 2024 •

edited

Loading

gaffer01 Jan 17, 2024

Issue 1668 - Check files have the correct jobId assigned to them in compaction job state store update #1684

Issue 1668 - Check files have the correct jobId assigned to them in compaction job state store update #1684

Conversation

kr565370 commented Jan 15, 2024 • edited Loading

Issue

Tests

Documentation

gaffer01 Jan 17, 2024

Choose a reason for hiding this comment

kr565370 commented Jan 15, 2024 •

edited

Loading