Skip to content

Make ParallelFieldGenerationRaceTest fail deterministically via CyclicBarrier#1233

Draft
Copilot wants to merge 23 commits into
masterfrom
copilot/add-parallel-execution-option
Draft

Make ParallelFieldGenerationRaceTest fail deterministically via CyclicBarrier#1233
Copilot wants to merge 23 commits into
masterfrom
copilot/add-parallel-execution-option

Conversation

Copilot AI commented May 20, 2026

Copy link
Copy Markdown
Contributor

The race test relied on OS thread scheduling to produce a two-enum write after a three-enum write, causing it to pass occasionally (false negative).

Changes

  • Barrier-coordinated writes — override createOutputStream in an anonymous MessageCodeGenerator subclass; install a CyclicBarrier(2) per output-file path (stored in a ConcurrentHashMap). The returned FilterOutputStream calls barrier.await() on the first write(), forcing two threads to be simultaneously mid-write on the same file on every run.
  • Separate generator for the golden baseline — the barrier requires two parties, so a single-task run would deadlock. A plain new MessageCodeGenerator() is used exclusively for the three-enum golden source.
  • Resilient barrier handlingBrokenBarrierException and TimeoutException are caught so an unexpected odd-writer-count never hangs the test; those threads proceed and still exercise the race.
CyclicBarrier barrier = barriers.computeIfAbsent(
        outputFile.getAbsolutePath(), k -> new CyclicBarrier(2));
return new FilterOutputStream(super.createOutputStream(outputFile)) {
    private boolean awaited = false;

    @Override
    public void write(byte[] b, int off, int len) throws IOException {
        awaitBarrierOnce();   // block until a second thread is also mid-write
        out.write(b, off, len);
    }
    ...
};

With 16 tasks (8 two-enum, 8 three-enum) writing 1000 field files, the barrier guarantees synchronized pairs on every file — the probability of all 1000 files ending with three-enum content becomes negligible, turning a probabilistic race into a near-certain failure.

@chrjohn

chrjohn commented May 20, 2026

Copy link
Copy Markdown
Member

@copilot please add logging output how many parallel tasks are used

Copilot AI commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please add logging output how many parallel tasks are used

Implemented in commit fde52ce. MessageCodeGenerator.generate(List<Task>) now logs the parallel worker count and total task count when parallel execution is enabled.

Copilot AI requested a review from chrjohn May 20, 2026 18:24
@chrjohn

chrjohn commented May 21, 2026

Copy link
Copy Markdown
Member

@copilot make generator.parallelExecution true by default.

Copilot AI commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot make generator.parallelExecution true by default.

Done in commit d9804f4. generator.parallelExecution is now true by default for multi-task generation; setting it to false forces sequential execution.

Copilot AI changed the title Enable optional parallel MessageCodeGenerator task execution with thread-safe state and task-scoped logging Enable parallel MessageCodeGenerator task execution by default with thread-safe state and task-scoped logging May 21, 2026
@chrjohn

chrjohn commented May 21, 2026

Copy link
Copy Markdown
Member

@copilot parallel execution might work when calling main() of the code generator. However, it does not work when using the GenerateMojo. So the mojo needs the information for all possible executions for the various fix versions at once. Currently there are several executions in quickfixj-messages/quickfixj-messages-all/pom.xml

Copilot AI commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot parallel execution might work when calling main() of the code generator. However, it does not work when using the GenerateMojo. So the mojo needs the information for all possible executions f...

Implemented in commit 3703f5f. GenerateMojo now accepts a tasks list and invokes generator.generate(List<Task>), and quickfixj-messages-all/pom.xml was switched from multiple generate executions to one aggregated execution containing all FIX-version tasks so parallel generation is applied across versions. No UI changes in this PR.

Copilot AI changed the title Enable parallel MessageCodeGenerator task execution by default with thread-safe state and task-scoped logging Enable parallel MessageCodeGenerator task execution by default, including GenerateMojo multi-version aggregation, with thread-safe state and task-scoped logging May 21, 2026
@chrjohn

chrjohn commented May 21, 2026

Copy link
Copy Markdown
Member

@copilot looking good so far.

  1. Please add a log entry when processing of one task starts and print the number of the task and the total number of tasks like ... "Started task for FIX44 (x / y)"
  2. and add a log entry when one task finishes and print the number of the task and the total number of tasks like ... "Finished task for FIX44 (x / y)"

Copilot AI commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot looking good so far.

  1. Please add a log entry when processing of one task starts and print the number of the task and the total number of tasks like ... "Started task for FIX44 (x / y)"
    2....

Implemented in commit 1804050. Added per-task progress logs in MessageCodeGenerator.generate(List<Task>): Started task for <taskName> (x / y) before execution and Finished task for <taskName> (x / y) in a finally block after each task completes (sequential and parallel modes). No UI changes in this PR, so no screenshot applies.

@chrjohn

chrjohn commented May 21, 2026

Copy link
Copy Markdown
Member

@copilot the test ParallelExecutionOptionTest.testParallelExecutionIsEnabledByDefault is flaky. It expects that the tasks always finish in the same order.

Keep the assertions that verify:

  • parallel execution actually happened, and
  • the “parallel task execution enabled” log was emitted.
  • Remove the assertion that requires task-3 to be the last finished task.
  • add a weaker assertion that any finish log for all 4 tasks exists.

Copilot AI changed the title Fix race test: parallel tasks must use separate output directories to detect divergence Add deterministic regression test for parallel IDSource.java corruption in shared output May 31, 2026
Copilot AI changed the title Add deterministic regression test for parallel IDSource.java corruption in shared output Rework ParallelFieldGenerationRaceTest to deterministic shared-output race reproducer Jun 1, 2026
Copilot AI changed the title Rework ParallelFieldGenerationRaceTest to deterministic shared-output race reproducer Narrow ParallelFieldGenerationRaceTest failure output to first divergent class Jun 1, 2026
File dictionary = tempFolder.newFile("RaceCondition-" + name + ".xml");
StringBuilder xml = new StringBuilder();
xml.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
xml.append("<fix major=\"4\" minor=\"4\">\n");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot one dictonary should have minor version 2, the other (withExtraEnum) should have minor version 4.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in commit 13e20b8. ParallelFieldGenerationRaceTest#createDictionary(...) now sets FIX minor version to 2 for the base dictionary and 4 for the withExtraEnum dictionary.

Copilot AI requested a review from chrjohn June 1, 2026 09:54
Comment thread pom.xml Outdated
Copilot AI changed the title Narrow ParallelFieldGenerationRaceTest failure output to first divergent class Make ParallelFieldGenerationRaceTest fail deterministically via CyclicBarrier Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants