[Issue 4153] Adds a backoff strategy to OpenAI API calls #3

mpetrini-larus · 2024-11-27T09:56:15Z

Fixes neo4j-contrib#4153
Adds a backoff strategy to OpenAI API calls.

Proposed Changes (Mandatory)

A brief list of proposed changes in order to fix the issue:

To better handle API failures caused by excessive load (HTTP 429 - Too Many Requests), the OpenAI API calls have been wrapped in a method that implements a backoff strategy.
This approach efficiently manages retries, reducing the likelihood of overwhelming the API and enhancing system resilience under high load conditions.

vga91

:emoji-tauromachia: giusto qualche modifichetta qua e la.

Oltre a quelle di sotto, metti pure la documentazione nel file docs/asciidoc/modules/ROOT/pages/ml/openai.adoc, nella tabella .Common configuration parameter,
quindi tipo

| enableBackOffRetries | <descrizione> (default: false)
| backOffRetries | <descrizione> (default: 5)
| exponentialBackoff | <descrizione> (default: false)
...

vga91 · 2024-11-27T10:07:19Z

extended/src/test/java/apoc/ml/OpenAIIT.java

@@ -36,7 +36,8 @@ public OpenAIIT() {

    @Before
    public void setUp() throws Exception {
-        openaiKey = System.getenv("OPENAI_KEY");
+        openaiKey = "OPENAI_KEY";
+//        openaiKey = System.getenv("OPENAI_KEY");


Questo si può togliere

vga91 · 2024-11-27T10:09:07Z

extended/src/main/java/apoc/ml/OpenAI.java

@@ -35,6 +37,8 @@ public class OpenAI {
    public static final String JSON_PATH_CONF_KEY = "jsonPath";
    public static final String PATH_CONF_KEY = "path";
    public static final String GPT_4O_MODEL = "gpt-4o";
+    public static final String ENABLE_BACK_OFF_RETRIES = "enableBackOffRetries";
+    public static final String BACK_OFF_RETRIES = "backOffRetries";


Suggested change

public static final String BACK_OFF_RETRIES = "backOffRetries";

public static final String BACK_OFF_RETRIES_CONF_KEY = "backOffRetries";

vga91 · 2024-11-27T10:22:26Z

extended/src/main/java/apoc/util/ExtendedUtil.java

+            Supplier<T> func, boolean retry, Integer backoffRetry,
+            Consumer<Exception> exceptionHandler
+    ){
+        return withBackOffRetries(func, retry, backoffRetry, exceptionHandler, Boolean.FALSE);


Forse se invece di Boolean mettiamo boolean si semplifica un po', perché non serve requireNonNullElse?

vga91 · 2024-11-27T10:27:21Z

extended/src/main/java/apoc/util/ExtendedUtil.java

+                long sleepMultiplier = Objects.requireNonNullElse(exponential, Boolean.FALSE) ?
+                        (long) Math.pow(2, backoffRetry - countDown) : // Exponential retry progression
+                        backoffRetry - countDown; // Linear retry progression
+                long delay = Math.min(
+                        Duration.ofSeconds(1)
+                                .multipliedBy(sleepMultiplier)
+                                .toMillis(),
+                        Duration.ofSeconds(30).toMillis() // Max 30s
+                );


Lo raggrupperei in un metodo, quindi farei così:

long delay = getDelay(backoffRetry, exponential, countDown);

e poi

private static long getDelay(Integer backoffRetry, Boolean exponential, Integer countDown) { long sleepMultiplier = Objects.requireNonNullElse(exponential, Boolean.FALSE) ? (long) Math.pow(2, backoffRetry - countDown) : // Exponential retry progression backoffRetry - countDown; // Linear retry progression long delay = Math.min( Duration.ofSeconds(1) .multipliedBy(sleepMultiplier) .toMillis(), Duration.ofSeconds(30).toMillis() // Max 30s ); return delay; }

vga91 · 2024-11-27T10:29:45Z

extended/src/main/java/apoc/ml/OpenAI.java

@@ -58,6 +62,8 @@ public EmbeddingResult(long index, String text, List<Double> embedding) {

    static Stream<Object> executeRequest(String apiKey, Map<String, Object> configuration, String path, String model, String key, Object inputs, String jsonPath, ApocConfig apocConfig, URLAccessChecker urlAccessChecker) throws JsonProcessingException, MalformedURLException {
        apiKey = (String) configuration.getOrDefault(APIKEY_CONF_KEY, apocConfig.getString(APOC_OPENAI_KEY, apiKey));
+        Boolean enableBackOffRetries = (Boolean) configuration.getOrDefault(ENABLE_BACK_OFF_RETRIES, Boolean.TRUE);


Anche se non è proprio proprio una breaking-change, meglio stare tranquilli,
la metterei con default a false.

Dovrebbe funzionare se fai così:

Util.toBoolean( configuration.get(ENABLE_BACK_OFF_RETRIES_CONF_KEY) )

vga91 · 2024-11-27T10:52:19Z

extended/src/main/java/apoc/ml/OpenAI.java

@@ -35,6 +37,8 @@ public class OpenAI {
    public static final String JSON_PATH_CONF_KEY = "jsonPath";
    public static final String PATH_CONF_KEY = "path";
    public static final String GPT_4O_MODEL = "gpt-4o";
+    public static final String ENABLE_BACK_OFF_RETRIES = "enableBackOffRetries";


Suggested change

public static final String ENABLE_BACK_OFF_RETRIES = "enableBackOffRetries";

public static final String ENABLE_BACK_OFF_RETRIES_CONF_KEY = "enableBackOffRetries";

vga91 · 2024-11-27T11:05:24Z

extended/src/main/java/apoc/ml/OpenAI.java

@@ -58,6 +62,8 @@ public EmbeddingResult(long index, String text, List<Double> embedding) {

    static Stream<Object> executeRequest(String apiKey, Map<String, Object> configuration, String path, String model, String key, Object inputs, String jsonPath, ApocConfig apocConfig, URLAccessChecker urlAccessChecker) throws JsonProcessingException, MalformedURLException {
        apiKey = (String) configuration.getOrDefault(APIKEY_CONF_KEY, apocConfig.getString(APOC_OPENAI_KEY, apiKey));
+        Boolean enableBackOffRetries = (Boolean) configuration.getOrDefault(ENABLE_BACK_OFF_RETRIES, Boolean.TRUE);
+        Integer backOffRetries = (Integer) configuration.getOrDefault(BACK_OFF_RETRIES, 5);


Visto che sta pagato, metterei anche un

boolean exponentialBackoff = Util.toBoolean( configuration.get(BACK_OFF_EXPONENTIAL_CONF_KEY) )

vga91 · 2024-11-27T11:10:12Z

extended/src/test/java/apoc/util/ExtendedUtilTest.java

+                )
+        );
+        long time = System.currentTimeMillis() - start;
+


Scrivi pure qua qualche riga di commento come sopra:

// The method will attempt to execute the operation with an exponential backoff strategy, ...etc

vga91

🥇 giusto queste due modifichine e siamo a cavallo.

vga91 · 2024-11-27T14:11:33Z

extended/src/main/java/apoc/ml/OpenAI.java

@@ -77,7 +85,7 @@ static Stream<Object> executeRequest(String apiKey, Map<String, Object> configur
        path = (String) configuration.getOrDefault(PATH_CONF_KEY, path);
        OpenAIRequestHandler apiType = type.get();

-        jsonPath = (String) configuration.getOrDefault(JSON_PATH_CONF_KEY, jsonPath);
+        String sJsonPath = (String) configuration.getOrDefault(JSON_PATH_CONF_KEY, jsonPath);


Farei anch'io un'altra variabile, ma visto che l'implementazione viene dall'alto meglio rimettere jsonPath come prima

si ma poi non lo posso passare alla lambda sotto 😕
se la lascio com'era non è "effettivamente-statica" e la lambda non la digerisce

Java maledetto, allora va bene così

extended/src/test/java/apoc/util/ExtendedUtilTest.java

Co-authored-by: Giuseppe Villani <[email protected]>

mpetrini-larus added 2 commits November 26, 2024 16:34

[issue_4153] Stub version of ExtendedUtil.withBackOffRetries

2635faa

[issue_4153] Adds a backoff strategy to OpenAI API calls

8d7874a

vga91 suggested changes Nov 27, 2024

View reviewed changes

[issue_4153] Fixes PR review issues

631d738

vga91 suggested changes Nov 27, 2024

View reviewed changes

Update extended/src/test/java/apoc/util/ExtendedUtilTest.java

2761f73

Co-authored-by: Giuseppe Villani <[email protected]>

vga91 approved these changes Nov 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Issue 4153] Adds a backoff strategy to OpenAI API calls #3

[Issue 4153] Adds a backoff strategy to OpenAI API calls #3

mpetrini-larus commented Nov 27, 2024

vga91 left a comment •

edited

Loading

vga91 Nov 27, 2024

vga91 Nov 27, 2024

vga91 Nov 27, 2024

vga91 Nov 27, 2024

vga91 Nov 27, 2024

vga91 Nov 27, 2024

vga91 Nov 27, 2024

vga91 Nov 27, 2024

vga91 left a comment

vga91 Nov 27, 2024

mpetrini-larus Nov 27, 2024 •

edited

Loading

vga91 Nov 27, 2024

	public static final String BACK_OFF_RETRIES = "backOffRetries";
	public static final String BACK_OFF_RETRIES_CONF_KEY = "backOffRetries";

	public static final String ENABLE_BACK_OFF_RETRIES = "enableBackOffRetries";
	public static final String ENABLE_BACK_OFF_RETRIES_CONF_KEY = "enableBackOffRetries";

[Issue 4153] Adds a backoff strategy to OpenAI API calls #3

Are you sure you want to change the base?

[Issue 4153] Adds a backoff strategy to OpenAI API calls #3

Conversation

mpetrini-larus commented Nov 27, 2024

Proposed Changes (Mandatory)

vga91 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vga91 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpetrini-larus Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vga91 left a comment •

edited

Loading

mpetrini-larus Nov 27, 2024 •

edited

Loading