Skip to content

Commit

Permalink
Fix hashing step ids in loops (#72)
Browse files Browse the repository at this point in the history
Per https://github.com/inngest/inngest/blob/main/docs/SDK_SPEC.md#512-ids-and-hashing,
add `:n` starting with `:1` for repeated instances of a step id

Refactored to be a combination of golang and inngest-js implementation to handle edge case of user defined stepId colliding
- https://github.com/inngest/inngestgo/blob/0a00daba0b2db68ff0f080f787cf63f0a63b44d8/internal/sdkrequest/manager.go#L123-L131
- https://github.com/inngest/inngest-js/blob/79069e1a3d700624ce49b323922c113fc952bcc6/packages/inngest/src/components/execution/v1.ts#L819-L831

* Use combination of hash and loop to find next unused stepId

The hash means we don't have to loop from 0 every time and for most
cases will just correctly return us the next unused stepId, but looping
afterwards guarantees we don't collide with a user defined stepId.

So this will be O(1) in most cases and potentially O(n) for pathological
functions that have many steps manually named with the `:n` suffix

* The inngest-js SDK currently optionally warns of parallel indexing, but
this isn't in scope for beta so I left it out.
  • Loading branch information
albertchae authored Sep 13, 2024
1 parent 2e01336 commit 5faeb64
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package com.inngest.springbootdemo.testfunctions;

import com.inngest.FunctionContext;
import com.inngest.InngestFunction;
import com.inngest.InngestFunctionConfigBuilder;
import com.inngest.Step;
import org.jetbrains.annotations.NotNull;

public class LoopFunction extends InngestFunction {

@NotNull
@Override
public InngestFunctionConfigBuilder config(InngestFunctionConfigBuilder builder) {
return builder
.id("loop-fn")
.name("Loop Function")
.triggerEvent("test/loop");
}


@Override
public Integer execute(FunctionContext ctx, Step step) {
int runningCount = 10;

// explicitly naming a step that the SDK will try to use in the loop shouldn't break the loop
int effectivelyFinalVariableForLambda1 = runningCount;
runningCount = step.run("add-num:3", () -> effectivelyFinalVariableForLambda1 + 50, Integer.class);

for (int i = 0; i < 5; i++) {
int effectivelyFinalVariableForLambda2 = runningCount;
// The actual stepIds used will be add-num, add-num:1, add-num:2, add-num:4, add-num:5
runningCount = step.run("add-num", () -> effectivelyFinalVariableForLambda2 + 10, Integer.class);
}

// explicitly reusing step names that the SDK used during the loop should both execute
// These will be modified to add-num:4:1 and add-num:4:2 respectively
int effectivelyFinalVariableForLambda3 = runningCount;
runningCount = step.run("add-num:4", () -> effectivelyFinalVariableForLambda3 + 30, Integer.class);
int effectivelyFinalVariableForLambda4 = runningCount;
runningCount = step.run("add-num:4", () -> effectivelyFinalVariableForLambda4 + 30, Integer.class);

return runningCount;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ protected HashMap<String, InngestFunction> functions() {
addInngestFunction(functions, new Scale2DObjectFunction());
addInngestFunction(functions, new MultiplyMatrixFunction());
addInngestFunction(functions, new WithOnFailureFunction());
addInngestFunction(functions, new LoopFunction());

return functions;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.inngest.springbootdemo;

import com.inngest.Inngest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ExecutionMode;
import org.springframework.beans.factory.annotation.Autowired;

import static org.junit.jupiter.api.Assertions.assertEquals;

@IntegrationTest
@Execution(ExecutionMode.CONCURRENT)
class LoopFunctionIntegrationTest {
@Autowired
private DevServerComponent devServer;

@Autowired
private Inngest client;

@Test
void testStepsInLoopExecuteCorrectly() throws Exception {
String loopEvent = InngestFunctionTestHelpers.sendEvent(client, "test/loop").getIds()[0];
Thread.sleep(2000);

RunEntry<Object> loopRun = devServer.runsByEvent(loopEvent).first();
assertEquals("Completed", loopRun.getStatus());

assertEquals(170, loopRun.getOutput());
}
}
27 changes: 26 additions & 1 deletion inngest/src/main/kotlin/com/inngest/State.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,14 @@ class StateNotFound : Throwable("State not found for id")
class State(
private val payloadJson: String,
) {
private val stepIdsToNextStepNumber = mutableMapOf<String, Int>()
private val stepIds = mutableSetOf<String>()

fun getHashFromId(id: String): String {
val bytes = id.toByteArray(Charsets.UTF_8)
val idToHash: String = findNextAvailableStepId(id)
stepIds.add(idToHash)

val bytes = idToHash.toByteArray(Charsets.UTF_8)
val digest = MessageDigest.getInstance("SHA-1")
val hashedBytes = digest.digest(bytes)
val sb = StringBuilder()
Expand All @@ -20,6 +26,25 @@ class State(
return sb.toString()
}

private fun findNextAvailableStepId(id: String): String {
if (id !in stepIds) {
return id
}

// start with the seen count so far for current stepId
// but loop over all seen stepIds to make sure a user didn't explicitly define
// a step using the same step number
var stepNumber = stepIdsToNextStepNumber.getOrDefault(id, 1)
while ("$id:$stepNumber" in stepIds) {
stepNumber = stepNumber + 1
}
// now we know stepNumber is unused and can be used for the current stepId
// save stepNumber + 1 to the hash for next time
stepIdsToNextStepNumber[id] = stepNumber + 1

return "$id:$stepNumber"
}

inline fun <reified T> getState(
hashedId: String,
fieldName: String = "data",
Expand Down

0 comments on commit 5faeb64

Please sign in to comment.