chia7712 commented on code in PR #17957:
URL: https://github.com/apache/kafka/pull/17957#discussion_r1866657271
##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -380,6 +380,14 @@ public CompletableFuture<Void> maybeInitialize() {
// Update state to initializing to avoid any concurrent requests
to be processed.
partitionState = SharePartitionState.INITIALIZING;
+ } catch (Exception e) {
+ log.error("Failed to initialize the share partition: {}-{}",
groupId, topicIdPartition, e);
+ completeInitializationWithException(future, e);
+ return future;
+ } finally {
+ lock.writeLock().unlock();
+ }
+ try {
Review Comment:
> I think the try catch in persister.readState was added as an additional
check in the initial iterations of development
maybe we can move the "additional check" into
`persister.readState/persister.writeState`? That can ensure they don't throw
exception directly. For example: `validate(request)` could cause exception.
> we don't seem to need it now. If we're all aligned, then I'll make this
change and re-raise a review.
yes, we don't need the `try-catch` if
`persister.readState/persister.writeState` always propagate exception by
`CompletableFuture`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
