Java sdk batchCheck does not do a batch check

[lucasmarcelli]

Checklist

• I have looked into the README and have not found a suitable solution or answer.
• I have looked into the documentation and have not found a suitable solution or answer.
• I have searched the issues and have not found a suitable solution or answer.
• I have upgraded to the latest version of OpenFGA and the issue still persists.
• I have searched the Slack community and have not found a suitable solution or answer.
• I agree to the terms within the OpenFGA Code of Conduct.

Description

Not sure if this is a bug per se, but the batchCheck function is not hitting the batch check endpoint on FGA. Rather, it's making several individual requests in parallel. If we make 30 checks in a batch check, with the default settings, it makes up to 10 parallel requests of single batch checks at a time until all 30 are checked.

We were having performance issues and noticed that this is happening, with context timeouts being hit after the 14th or 15th tuple check and the rest coming back null. We were able to mitigate this by increasing our db memory.

Is the way this works intentional? I'm planning to try hitting the actual batch endpoint in openfga manually to see if the performance is improved at all, but wanted to check here as well.

java-sdk/src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java

Lines 582 to 621 in 5713a50

	public CompletableFuture<List<ClientBatchCheckResponse>> batchCheck(
	List<ClientCheckRequest> requests, ClientBatchCheckOptions batchCheckOptions)
	throws FgaInvalidParameterException {
	configuration.assertValid();
	configuration.assertValidStoreId();
	var options = batchCheckOptions != null
	? batchCheckOptions
	: new ClientBatchCheckOptions().maxParallelRequests(DEFAULT_MAX_METHOD_PARALLEL_REQS);
	if (options.getAdditionalHeaders() == null) {
	options.additionalHeaders(new HashMap<>());
	}
	options.getAdditionalHeaders().putIfAbsent(CLIENT_METHOD_HEADER, "BatchCheck");
	options.getAdditionalHeaders()
	.putIfAbsent(CLIENT_BULK_REQUEST_ID_HEADER, randomUUID().toString());
	int maxParallelRequests = options.getMaxParallelRequests() != null
	? options.getMaxParallelRequests()
	: DEFAULT_MAX_METHOD_PARALLEL_REQS;
	var executor = Executors.newScheduledThreadPool(maxParallelRequests);
	var latch = new CountDownLatch(requests.size());
	var responses = new ConcurrentLinkedQueue<ClientBatchCheckResponse>();
	final var clientCheckOptions = options.asClientCheckOptions();
	Consumer<ClientCheckRequest> singleClientCheckRequest =
	request -> call(() -> this.check(request, clientCheckOptions))
	.handleAsync(ClientBatchCheckResponse.asyncHandler(request))
	.thenAccept(responses::add)
	.thenRun(latch::countDown);
	try {
	requests.forEach(request -> executor.execute(() -> singleClientCheckRequest.accept(request)));
	latch.await();
	return CompletableFuture.completedFuture(new ArrayList<>(responses));
	} catch (Exception e) {
	return CompletableFuture.failedFuture(e);
	}
	}

Expectation

batchCheck should hit the batch endpoint in openfga.

Reproduction

1. Make a batch check
2. Observe that the URL being hit by the package is not the batch check endpoint, but is multiple requests to the single check endpoint

OpenFGA SDK version

0.4.x

OpenFGA version

1.5.9

SDK Configuration

i'm not sure what this means

Logs

No response

References

No response

[ewanharris]

Hey @lucasmarcelli,

We're currently in the process of working to update the SDKs to use the BatchCheck API endpoint. So, the BatchCheck method is currently still the SDK wrapper method that makes multiple Check calls in parallel.

[lucasmarcelli]

Thanks. Is there a task/issue?

[lucasmarcelli]

I ask because I may have capacity to make this change in the java SDK if nobody has picked it up yet.

[ewanharris]

@lucasmarcelli thanks for that offer!

We don't currently have this work broken down yet as we're finalizing our plans for it (we plan to maintain the same API interface and map the request/response internally). But I've raised your interest to the team so that we can be sure to let you know when we have that issue filed.

[jimmyjames]

If anyone is interested in contributing, the pattern we are following for the new server batch check is:

• Rename the existing batchCheck method to clientBatchCheck, to be available for those who wish (or need) to continue to use the existing method
• Rename the current batch check types to indicate it is a client (not API) feature. It's not perfect but the pattern we have been going with is to add Client to the types. e.g., ClientBatchCheckResponse becomes ClientBatchCheckClientResponse.
• Generate the API models using the sdk generator (make build-client-java).
• Create client wrappers types for the request and response. You can see the shape in the JS change or the Python change
• The new batchCheck method should accept optional params for both the maximum parallel requests (default remains the same) and maximum batch size (default of 50). If the number of checks exceeds the batch size, the requests should be parallelized up to the max parallel requests.

I know that's a lot of info 😅, it may be easier to refer to the JS or Python PRs to see the pattern :)

If anyone would like to pick this up before we get to it, feel free to assign this issue to yourself and we can assist as needed!

[piotrooo]

@jimmyjames This could probably be closed in favor of the latest changes.