github-issue#2377#2378
Open
dhanendra06 wants to merge 5 commits into
Open
Conversation
Signed-off-by: Dhanendra Sahu <dhanendra.tech@gmail.com>
Signed-off-by: Dhanendra Sahu <dhanendra.tech@gmail.com>
Signed-off-by: Dhanendra Sahu <dhanendra.tech@gmail.com>
|
Warning Review limit reached
More reviews will be available in 35 minutes and 46 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughMultiple registration processor stages now reuse the ChangesRouter reuse wiring update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@registration-processor/core-processor/registration-processor-verification-stage/src/main/java/io/mosip/registration/processor/verification/stage/VerificationStage.java`:
- Line 179: The health-check router setup in VerificationStage.start() is using
the wrong MessageBusAddress constants, which makes it point at the manual
adjudication bus instead of the verification bus. Update the postUrl(getVertx(),
...) call to use the same VERIFICATION_BUS_IN and VERIFICATION_BUS_OUT symbols
already used elsewhere in VerificationStage so the health check monitors the
correct channels.
In
`@registration-processor/pre-processor/registration-processor-securezone-notification-stage/src/main/java/io/mosip/registration/processor/securezone/notification/stage/SecurezoneNotificationStage.java`:
- Around line 131-133: The route registration in SecurezoneNotificationStage is
still using the shared MosipRouter singleton, so another verticle can swap the
internal router before routes() finishes and break the notification endpoint.
Update the stage to register the `/notification` route directly on the local
vertxRouter instance instead of calling router.setRoute(...) and
this.routes(router), and keep the handler/failureHandler wiring in
SecurezoneNotificationStage so route binding is local and atomic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 262a9737-54d4-429d-a78c-cf1aab16f615
📒 Files selected for processing (22)
registration-processor/core-processor/registration-processor-abis-handler-stage/src/main/java/io/mosip/registration/processor/abis/handler/stage/AbisHandlerStage.javaregistration-processor/core-processor/registration-processor-abis-middleware-stage/src/main/java/io/mosip/registartion/processor/abis/middleware/stage/AbisMiddleWareStage.javaregistration-processor/core-processor/registration-processor-bio-dedupe-stage/src/main/java/io/mosip/registration/processor/biodedupe/stage/BioDedupeStage.javaregistration-processor/core-processor/registration-processor-biometric-authentication-stage/src/main/java/io/mosip/registration/processor/biometric/authentication/stage/BiometricAuthenticationStage.javaregistration-processor/core-processor/registration-processor-biometric-extraction-stage/src/main/java/io/mosip/registration/processor/stages/biometric/extraction/stage/BiometricExtractionStage.javaregistration-processor/core-processor/registration-processor-demo-dedupe-stage/src/main/java/io/mosip/registration/processor/stages/demodedupe/DemoDedupeStage.javaregistration-processor/core-processor/registration-processor-finalization-stage/src/main/java/io/mosip/registration/processor/stages/finalization/stage/FinalizationStage.javaregistration-processor/core-processor/registration-processor-manual-adjudication-stage/src/main/java/io/mosip/registration/processor/adjudication/stage/ManualAdjudicationStage.javaregistration-processor/core-processor/registration-processor-uin-generator-stage/src/main/java/io/mosip/registration/processor/stages/uingenerator/stage/UinGeneratorStage.javaregistration-processor/core-processor/registration-processor-verification-stage/src/main/java/io/mosip/registration/processor/verification/stage/VerificationStage.javaregistration-processor/init/registration-processor-packet-receiver-stage/src/main/java/io/mosip/registration/processor/packet/receiver/stage/PacketReceiverStage.javaregistration-processor/post-processor/registration-processor-credential-requestor-stage/src/main/java/io/mosip/registration/processor/credentialrequestor/stage/CredentialRequestorStage.javaregistration-processor/post-processor/registration-processor-message-sender-stage/src/main/java/io/mosip/registration/processor/message/sender/stage/MessageSenderStage.javaregistration-processor/pre-processor/registration-processor-cmd-validator-stage/src/main/java/io/mosip/registration/processor/stages/app/CMDValidatorStage.javaregistration-processor/pre-processor/registration-processor-introducer-validator-stage/src/main/java/io/mosip/registration/processor/stages/introducervalidator/IntroducerValidatorStage.javaregistration-processor/pre-processor/registration-processor-operator-validator-stage/src/main/java/io/mosip/registration/processor/stages/operatorvalidator/OperatorValidatorStage.javaregistration-processor/pre-processor/registration-processor-packet-classifier-stage/src/main/java/io/mosip/registration/processor/stages/packetclassifier/PacketClassifierStage.javaregistration-processor/pre-processor/registration-processor-packet-uploader-stage/src/main/java/io/mosip/registration/processor/packet/uploader/stage/PacketUploaderStage.javaregistration-processor/pre-processor/registration-processor-packet-validator-stage/src/main/java/io/mosip/registration/processor/stages/packet/validator/PacketValidatorStage.javaregistration-processor/pre-processor/registration-processor-quality-classifier-stage/src/main/java/io/mosip/registration/processor/quality/classifier/stage/QualityClassifierStage.javaregistration-processor/pre-processor/registration-processor-securezone-notification-stage/src/main/java/io/mosip/registration/processor/securezone/notification/stage/SecurezoneNotificationStage.javaregistration-processor/pre-processor/registration-processor-supervisor-validator-stage/src/main/java/io/mosip/registration/processor/stages/supervisorvalidator/SupervisorValidatorStage.java
| public void start() { | ||
| router.setRoute(this.postUrl(getVertx(), MessageBusAddress.MANUAL_ADJUDICATION_BUS_IN, MessageBusAddress.MANUAL_ADJUDICATION_BUS_OUT)); | ||
| this.createServer(router.getRouter(), getPort()); | ||
| io.vertx.ext.web.Router vertxRouter = this.postUrl(getVertx(), MessageBusAddress.MANUAL_ADJUDICATION_BUS_IN, MessageBusAddress.MANUAL_ADJUDICATION_BUS_OUT); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'VERIFICATION_BUS_(IN|OUT)|MANUAL_ADJUDICATION_BUS_(IN|OUT)' \
registration-processor/core-processor/registration-processor-verification-stage/src/main/java/io/mosip/registration/processor/verification/stage/VerificationStage.javaRepository: mosip/registration
Length of output: 509
🏁 Script executed:
#!/bin/bash
rg -nP 'VERIFICATION_BUS_IN|VERIFICATION_BUS_OUT|MANUAL_ADJUDICATION_BUS_IN|MANUAL_ADJUDICATION_BUS_OUT' \
registration-processor/core-processor/registration-processor-verification-stage/src/main/java/io/mosip/registration/processor/verification/stage/VerificationStage.javaRepository: mosip/registration
Length of output: 509
Incorrect MessageBusAddress constants in start()
Line 179 configures the health-check router using MANUAL_ADJUDICATION_BUS_IN and MANUAL_ADJUDICATION_BUS_OUT. This is VerificationStage, and the rest of the class (lines 158 and 190) correctly uses VERIFICATION_BUS_IN and VERIFICATION_BUS_OUT. Using the wrong addresses will cause the health check to monitor the incorrect message bus channels.
🐛 Proposed fix
- io.vertx.ext.web.Router vertxRouter = this.postUrl(getVertx(), MessageBusAddress.MANUAL_ADJUDICATION_BUS_IN, MessageBusAddress.MANUAL_ADJUDICATION_BUS_OUT);
+ io.vertx.ext.web.Router vertxRouter = this.postUrl(getVertx(), MessageBusAddress.VERIFICATION_BUS_IN, MessageBusAddress.VERIFICATION_BUS_OUT);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| io.vertx.ext.web.Router vertxRouter = this.postUrl(getVertx(), MessageBusAddress.MANUAL_ADJUDICATION_BUS_IN, MessageBusAddress.MANUAL_ADJUDICATION_BUS_OUT); | |
| io.vertx.ext.web.Router vertxRouter = this.postUrl(getVertx(), MessageBusAddress.VERIFICATION_BUS_IN, MessageBusAddress.VERIFICATION_BUS_OUT); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@registration-processor/core-processor/registration-processor-verification-stage/src/main/java/io/mosip/registration/processor/verification/stage/VerificationStage.java`
at line 179, The health-check router setup in VerificationStage.start() is using
the wrong MessageBusAddress constants, which makes it point at the manual
adjudication bus instead of the verification bus. Update the postUrl(getVertx(),
...) call to use the same VERIFICATION_BUS_IN and VERIFICATION_BUS_OUT symbols
already used elsewhere in VerificationStage so the health check monitors the
correct channels.
Signed-off-by: Dhanendra Sahu <dhanendra.tech@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@registration-processor/registration-processor-core/src/main/java/io/mosip/registration/processor/core/abstractverticle/MosipRouter.java`:
- Around line 61-64: The new MosipRouter.post(Router, String) overload still
writes the created Route into shared mutable state, so the race remains because
handler(...) and nonSecureHandler(...) later read this.route. Update MosipRouter
so route binding is done on the Route returned from post(...) instead of storing
it in the singleton field; make handler/nonSecureHandler accept a Route
parameter or otherwise operate on the local Route instance. Then adjust the
consuming stage routes (for example PacketReceiverStage.routes(...) and
SecurezoneNotificationStage.routes(...)) to capture the returned Route and chain
handlers on that same instance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8f9bd621-e8e2-4376-a800-ff3b820e7113
📒 Files selected for processing (6)
registration-processor/core-processor/registration-processor-verification-stage/src/main/java/io/mosip/registration/processor/verification/stage/VerificationStage.javaregistration-processor/init/registration-processor-packet-receiver-stage/src/main/java/io/mosip/registration/processor/packet/receiver/stage/PacketReceiverStage.javaregistration-processor/pre-processor/registration-processor-packet-classifier-stage/src/main/java/io/mosip/registration/processor/stages/config/PacketClassifierConfig.javaregistration-processor/pre-processor/registration-processor-packet-classifier-stage/src/main/java/io/mosip/registration/processor/stages/packetclassifier/PacketClassifierStage.javaregistration-processor/pre-processor/registration-processor-securezone-notification-stage/src/main/java/io/mosip/registration/processor/securezone/notification/stage/SecurezoneNotificationStage.javaregistration-processor/registration-processor-core/src/main/java/io/mosip/registration/processor/core/abstractverticle/MosipRouter.java
💤 Files with no reviewable changes (3)
- registration-processor/pre-processor/registration-processor-packet-classifier-stage/src/main/java/io/mosip/registration/processor/stages/config/PacketClassifierConfig.java
- registration-processor/pre-processor/registration-processor-packet-classifier-stage/src/main/java/io/mosip/registration/processor/stages/packetclassifier/PacketClassifierStage.java
- registration-processor/core-processor/registration-processor-verification-stage/src/main/java/io/mosip/registration/processor/verification/stage/VerificationStage.java
Signed-off-by: Dhanendra Sahu <dhanendra.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem 1 — Slow Startup (@RefreshScope)
MessageSenderStage had @RefreshScope alongside @service and @configuration. This made Spring defer the actual bean initialization (including scanning 8+ component packages) until first use on the Vert.x
event loop thread. That blocked the event loop during Vert.x deployment, making all three stages slow to start — taking 3–4 minutes instead of under a minute.
Impact: Kubernetes initialDelaySeconds was tight. The slow startup was pushing readiness probe responses past the deadline, triggering pod restarts before the application was even ready.
Fix: Removed @RefreshScope from MessageSenderStage. Spring now eagerly initializes the bean during context startup. All three stages are up and ready in ~43 seconds.
Problem 2 — Health Check 404s (MosipRouter Race Condition)
MosipRouter is a Spring @component singleton — one shared instance for the entire JVM. All three stages @Autowired the same bean and their start() methods ran concurrently (each stage on its own Vert.x
event loop thread).
Every stage's start() followed this pattern:
router.setRoute(this.postUrl(...)) // writes THIS stage's router into the shared singleton
this.createServer(router.getRouter(), getPort()) // reads WHOEVER's router is there NOW
With concurrent execution, a stage could write its router into the singleton and then read back a different stage's router (written by another thread in between). That stage's HTTP server then started with
the wrong Vert.x Router — one that didn't have its health endpoint path registered. Kubernetes liveness probes hitting that port got 404, failed repeatedly, and killed the pod.
This explained why the failure was non-deterministic: log 1 showed securezone healthy but quality-classifier failing; log 2 (after restart) showed securezone failing too. Thread scheduling determined the
winner each time.
Fix: In each stage's start(), the Vert.x Router returned by postUrl() is captured in a local variable and passed directly to createServer():
// Before — race window between these two lines:
router.setRoute(this.postUrl(...));
this.createServer(router.getRouter(), getPort());
// After — no race; same object used end-to-end:
io.vertx.ext.web.Router vertxRouter = this.postUrl(...);
router.setRoute(vertxRouter);
this.createServer(vertxRouter, getPort());
The health endpoint is registered inside postUrl() on vertxRouter. Passing vertxRouter directly to createServer() guarantees each stage's HTTP server always has its own correct health routes — regardless of
what other threads do to the shared MosipRouter singleton.
Summary by CodeRabbit
Refactor
Configuration