quarkiverse / quarkus-langchain4j

Quarkus Langchain4j extension
https://docs.quarkiverse.io/quarkus-langchain4j/dev/index.html
Apache License 2.0
121 stars 66 forks source link

Setting max-retries=0 causes infinite retries #482

Open jmartisk opened 3 months ago

jmartisk commented 3 months ago

If you set, say, quarkus.langchain4j.openai.max-retries=0, it will attempt to retry indefinitely. The correct setting for "no retries" is 1, but given the property name, some people might think that max-retries=0 means "just try once", which isn't the case. Instead, it causes the retry loop to immediately break through the upper bound, and because the end condition is based on == it will probably only stop after an integer overflow...

jmartisk commented 3 months ago

I'll probably fix this in two places, once in the upstream class dev.langchain4j.internal.RetryUtils to reject 0 max-retries, and also at the level of validating Quarkus configuration.

geoand commented 3 months ago

To be honest, I think we should not retry at all by default and promote the use of Smallrye Fault Tolerance.

But in any case max-retries=0 should indeed not do infinite retries

jmartisk commented 3 months ago

To be honest, I think we should not retry at all by default and promote the use of Smallrye Fault Tolerance.

How would one do that, annotate some application methods with MP FT annotations and set the max-retries to 1 to avoid the clashing of two approaches? The problem is that an application developer cannot annotate the methods of the ChatLanguageModel interface directly

geoand commented 3 months ago

My idea is that we should have retries on the LangChain4j level turned off by default and push people to use FT with AiService.

For ChatLanguageModel we obviously can't do anything of that sort.

jmartisk commented 3 months ago

Ok, so perhaps I could send a PR that turns off the LangChain4j-level retries for chat models by default (change the default to 1) and make it clear in the configuration javadocs that if one uses AiServices, the preferred way to achieve FT is to use MP FT annotations on the AI service?

(This may not be reasonable to do in some cases - for example, if some of the used tools aren't idempotent)

geoand commented 3 months ago

Sounds like a plan!