Skip to content

docBatchSize warning is confusing #1291

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.

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

Closed
rjrudin opened this issue Mar 8, 2021 · 9 comments
Closed

docBatchSize warning is confusing #1291

rjrudin opened this issue Mar 8, 2021 · 9 comments

Comments

@rjrudin
Copy link
Contributor

rjrudin commented Mar 8, 2021

So we can address your issue, please include the following:

Version of MarkLogic Java Client API

5.4.0

Version of MarkLogic Server

N/A

Java version

N/A

OS and version

N/A

Input: Some code to illustrate the problem, preferably in a state that can be independently reproduced on our end

DHF develop branch is using Java Client 5.4.0 now. DHF uses a QueryBatcher to run a flow. It constructs a QueryBatcher based on an Iterator on a Java collection. Every time I run a flow now, I see the following logged at the warning level:

"docBatchSize is beyond maxDocBatchSize"

In my test, the Java collection used to build the iterator has 6 items in it. I get this warning no matter what I set the batch size to - 1, 3, 6, 30, 300.

The main issue is that because this is logged at the warn level, it always shows up when a user uses Gradle to run a flow. The message doesn't have any context, nor does the user know what either of those values are (particularly 'maxDocBatchSize'), so the user doesn't know what to do about it.

What that means is if DHF 5.5 is released and this warning shows up, the DHF team will be getting questions about why this is showing up and what users ought to do about it. And we don't know.

Actual output: What did you observe? What errors did you see? Can you attach the logs? (Java logs, MarkLogic logs)

Expected output: What specifically did you expect to happen?

Given that I don't know what to do with this message, my first though is - don't show it. Or log it at the debug level. But it likely needs to be improved to provide more context, and it also needs to be written in a more user-facing way - e.g. "The batchSize is greater than (some explanation of what maxDocBatchSize refers to); consider (doing something that likely makes more sense than what the user is currently doing)".

Alternatives: What else have you tried, actual/expected?

@rjrudin
Copy link
Contributor Author

rjrudin commented Apr 30, 2021

@ehennum @llinggit Could this be addressed in a 5.4.x release? I'm concerned about this going into the DHF 5.5.0 release and the avalanche of questions users will have about "Why am I getting 'docBatchSize is beyond maxDocBatchSize' warnings? What is wrong with DHF 5.5.0??? This wasn't happening before!!"

@llinggit
Copy link
Contributor

llinggit commented Jun 9, 2021

Hi @rjrudin, I'll try to hide this error msg to a lower level first. At the same time, could you share with us which server version you are using, please? Thanks.

@rjrudin
Copy link
Contributor Author

rjrudin commented Jun 9, 2021

It was ML 10.0-6.x

@llinggit
Copy link
Contributor

llinggit commented Jun 9, 2021

Thanks.

llinggit pushed a commit to llinggit/java-client-api that referenced this issue Jun 10, 2021
llinggit pushed a commit that referenced this issue Jun 10, 2021
@llinggit llinggit assigned georgeajit and unassigned llinggit Jun 10, 2021
@llinggit llinggit added test and removed new labels Jun 10, 2021
ehennum pushed a commit that referenced this issue Jul 2, 2021
@georgeajit georgeajit added verify and removed test labels Jul 7, 2021
@georgeajit georgeajit assigned llinggit and unassigned georgeajit Jul 7, 2021
@georgeajit
Copy link

Can we remove the commented overloaded method - one with throttle factor in QueryBatcher class?

//public QueryBatcher withBatchSize(int docBatchSize, int docToUriBatchRatio, int threadThrottleFactor);

@georgeajit
Copy link

Ran a test with ratio parameter included. Did not see any warning messages.

@georgeajit
Copy link

Can be shipped once stale code is stripped out.

llinggit pushed a commit to llinggit/java-client-api that referenced this issue Jul 13, 2021
@llinggit
Copy link
Contributor

There's no server-side change in this issue.

@llinggit llinggit added test and removed verify labels Jul 14, 2021
@llinggit llinggit removed their assignment Jul 14, 2021
@rjrudin
Copy link
Contributor Author

rjrudin commented Jul 20, 2021

This can be closed due to the 5.5.0 release, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants