Skip to content

Conversation

@rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Jan 5, 2026

No description provided.

Copilot AI review requested due to automatic review settings January 5, 2026 17:30
@github-actions
Copy link

github-actions bot commented Jan 5, 2026

Copyright Validation Results
Total: 3 | Passed: 3 | Failed: 0 | Skipped: 0 | at: 2026-01-05 18:11:33 UTC | commit: 3a27dff

✅ Valid Files

  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/impl/DataMovementServices.java
  • marklogic-client-api/src/test/java/com/marklogic/client/datamovement/FewerServerThreadsThanForestsTest.java
  • marklogic-client-api/src/test/java/com/marklogic/client/test/Common.java

✅ All files have valid copyright headers!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a server-side bug where maxDocToUriBatchRatio returns -1 when the number of server threads is less than the number of forests, preventing a LinkedBlockingQueue construction error due to negative capacity.

  • Added validation to default maxDocToUriBatchRatio to 1 when the server returns a value ≤ 0
  • Created a comprehensive test to verify the fix by simulating the condition where server threads are fewer than forests
  • Added utility methods in test infrastructure to support server configuration changes during testing

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
marklogic-client-api/src/main/java/com/marklogic/client/datamovement/impl/DataMovementServices.java Added validation to prevent negative maxDocToUriBatchRatio values by defaulting to 1
marklogic-client-api/src/test/java/com/marklogic/client/datamovement/FewerServerThreadsThanForestsTest.java Added test that reproduces the bug scenario by reducing server threads below forest count
marklogic-client-api/src/test/java/com/marklogic/client/test/Common.java Added newAdminManager() utility method to support server configuration changes in tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

queryBatcher.awaitCompletion();
dmm.stopJob(queryBatcher);

assertEquals(4, uriCount.get(), "Verifies that the 4 tet documents were found, and more importantly, " +
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

Corrected spelling of 'tet' to 'test'.

Suggested change
assertEquals(4, uriCount.get(), "Verifies that the 4 tet documents were found, and more importantly, " +
assertEquals(4, uriCount.get(), "Verifies that the 4 test documents were found, and more importantly, " +

Copilot uses AI. Check for mistakes.
BillFarber
BillFarber previously approved these changes Jan 5, 2026
@rjrudin rjrudin merged commit aa95e39 into develop Jan 5, 2026
3 checks passed
@rjrudin rjrudin deleted the feature/26460-bug-fix branch January 5, 2026 18:43
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.

3 participants