feat: add Microservices Bulkhead pattern (#3228) - #3597
Conversation
PR SummaryIntroduces a new microservices-bulkhead module implementing the Bulkhead pattern with per-downstream thread pools, fail-fast behavior, and testing. Includes App demo, Bulkhead core, services, tests, UML, and updated parent POM. Changes
autogenerated by presubmit.ai |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Files Processed (14)
- microservices-bulkhead/README.md (1 hunk)
- microservices-bulkhead/etc/microservices-bulkhead.urm.puml (1 hunk)
- microservices-bulkhead/pom.xml (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/App.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/Bulkhead.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/BulkheadFullException.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/InventoryService.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/PaymentService.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/RemoteService.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/AppTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/BulkheadTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/InventoryServiceTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/PaymentServiceTest.java (1 hunk)
- pom.xml (1 hunk)
Actionable Comments (2)
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/App.java [64-69]
readability: "Logger naming consistency with Lombok"
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/Bulkhead.java [105-110]
readability: "Logger naming inconsistency in Bulkhead"
Skipped Comments (0)
| LOGGER.info("--- Scenario 1: one shared thread pool for every downstream call ---"); | ||
| try (var sharedPool = new Bulkhead("shared-pool", 2, 2)) { | ||
| var paymentFutures = flood(sharedPool, payment, "order", 4); | ||
| callInventory(sharedPool, inventory, "order-5"); | ||
| awaitAll(paymentFutures); | ||
| } |
There was a problem hiding this comment.
The code uses LOGGER with Lombok @slf4j. Lombok typically exposes 'log' as the logger field. If LOGGER is not provided by Lombok in this project, update to the standard 'log' name or align with the project's logging convention to avoid potential compilation issues.
| LOGGER.debug( | ||
| "Bulkhead '{}' accepted call ({} active, {} queued)", | ||
| name, | ||
| executor.getActiveCount(), | ||
| executor.getQueue().size()); | ||
| return future; |
There was a problem hiding this comment.
The logger usage mirrors App.java: verify that the Lombok-provided logger is named LOGGER in this codebase. If Lombok exposes the field as 'log' by default, switch to log or configure Lombok to generate LOGGER to avoid compilation errors.
|
Note on the automated review comments: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3597 +/- ##
============================================
+ Coverage 83.79% 83.95% +0.16%
- Complexity 4277 4311 +34
============================================
Files 1121 1126 +5
Lines 15144 15253 +109
Branches 723 728 +5
============================================
+ Hits 12690 12806 +116
+ Misses 2159 2153 -6
+ Partials 295 294 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
94d0736 to
3ada5e2
Compare
|
Follow-up on the Codecov note: added five |
3ada5e2 to
9532148
Compare
|
The last CI run failed in |
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Files Processed (14)
- microservices-bulkhead/README.md (1 hunk)
- microservices-bulkhead/etc/microservices-bulkhead.urm.puml (1 hunk)
- microservices-bulkhead/pom.xml (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/App.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/Bulkhead.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/BulkheadFullException.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/InventoryService.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/PaymentService.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/RemoteService.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/AppTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/BulkheadTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/InventoryServiceTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/PaymentServiceTest.java (1 hunk)
- pom.xml (1 hunk)
Actionable Comments (4)
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/App.java [64-64]
bug: "Logger name mismatch with Lombok"
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/InventoryService.java [38-38]
bug: "Logger name mismatch in InventoryService"
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/PaymentService.java [51-51]
bug: "Logger name mismatch in PaymentService"
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/Bulkhead.java [105-111]
bug: "Logger name consistency in Bulkhead"
Skipped Comments (0)
| var payment = new PaymentService(PAYMENT_LATENCY); | ||
| var inventory = new InventoryService(); | ||
|
|
||
| LOGGER.info("--- Scenario 1: one shared thread pool for every downstream call ---"); |
There was a problem hiding this comment.
Logger field name mismatch with Lombok @slf4j. Lombok generates a logger named 'log' by default, but the code references LOGGER. This will fail to compile. Align to the project's logging convention (usually 'log') and update all references accordingly.
|
|
||
| @Override | ||
| public String call(String request) { | ||
| LOGGER.info("Inventory system received '{}'", request); |
There was a problem hiding this comment.
Logger field name mismatch with Lombok @slf4j. Same issue as App.java; update to use 'log' or ensure Lombok generates the expected field name.
|
|
||
| @Override | ||
| public String call(String request) { | ||
| LOGGER.info("Payment provider received '{}', it will take {} ms", request, latency.toMillis()); |
There was a problem hiding this comment.
Logger field name mismatch with Lombok @slf4j. Update to 'log' or standardize on the project's logger field name.
| LOGGER.debug( | ||
| "Bulkhead '{}' accepted call ({} active, {} queued)", | ||
| name, | ||
| executor.getActiveCount(), | ||
| executor.getQueue().size()); | ||
| return future; | ||
| } catch (RejectedExecutionException e) { |
There was a problem hiding this comment.
Logger usage in Bulkhead relies on a Lombok-generated 'log' field, but the code references LOGGER in a multi-line log call. Align to the Lombok-provided field name across the class to avoid compilation/runtime issues.
9532148 to
4cd5d56
Compare
|
Coverage follow-up: added |
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Files Processed (14)
- microservices-bulkhead/README.md (1 hunk)
- microservices-bulkhead/etc/microservices-bulkhead.urm.puml (1 hunk)
- microservices-bulkhead/pom.xml (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/App.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/Bulkhead.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/BulkheadFullException.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/InventoryService.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/PaymentService.java (1 hunk)
- microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/RemoteService.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/AppTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/BulkheadTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/InventoryServiceTest.java (1 hunk)
- microservices-bulkhead/src/test/java/com/iluwatar/bulkhead/PaymentServiceTest.java (1 hunk)
- pom.xml (1 hunk)
Actionable Comments (1)
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/App.java [60-69]
readability: "Logger field name mismtach with Lombok SLF4J"
Skipped Comments (2)
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/InventoryService.java [36-39]
readability: "Logger field naming mismatch in InventoryService"
-
microservices-bulkhead/src/main/java/com/iluwatar/bulkhead/PaymentService.java [50-59]
readability: "Logger field naming mismatch in PaymentService"
| public static void main(String[] args) { | ||
| var payment = new PaymentService(PAYMENT_LATENCY); | ||
| var inventory = new InventoryService(); | ||
|
|
||
| LOGGER.info("--- Scenario 1: one shared thread pool for every downstream call ---"); | ||
| try (var sharedPool = new Bulkhead("shared-pool", 2, 2)) { | ||
| var paymentFutures = flood(sharedPool, payment, "order", 4); | ||
| callInventory(sharedPool, inventory, "order-5"); | ||
| awaitAll(paymentFutures); | ||
| } |
There was a problem hiding this comment.
Logger naming: Lombok @slf4j generates a logger field (default name: log). The code uses LOGGER, which will not compile unless the logger field is explicitly named LOGGER. Either switch the usage to log or annotate with @slf4j(topic = "LOGGER") to rename the field. This will apply to all references in the App class where LOGGER.info is used.
1 participant
What does this PR do?
Adds the Microservices Bulkhead pattern as a new
microservices-bulkheadmodule.Bulkhead, a dedicated fixed-size thread pool with a bounded queue. When the compartment is full the call fails fast withBulkheadFullExceptioninstead of blocking or borrowing threads from other compartments.Bulkhead: named, boundedThreadPoolExecutor(threads + queue), fail-fast rejection, metrics (active, queued, rejected),AutoCloseable.BulkheadFullException: unchecked exception carrying the compartment name.RemoteService,PaymentService(slow),InventoryService(healthy): simulated downstream dependencies.App: runs the same load first through one shared pool (inventory call gets rejected) and then through dedicated bulkheads (payment overflow rejected fast, inventory keeps answering), with log output tracing every step.README.md: intent, real-world example, sequence diagram, code walkthrough, applicability, trade-offs, related patterns. PlantUML class diagram underetc/.AppTest.pom.xml../mvnw clean verify -pl microservices-bulkheadpasses locally on JDK 21 and inside aneclipse-temurin:21container.Fixes #3228