-
Notifications
You must be signed in to change notification settings - Fork 20
Kafka lag issue log line #882
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis update significantly enhances logging across multiple classes related to demand generation, improving traceability and debugging. Key information such as tenant IDs and billing periods is now captured, while redundant log entries have been removed. Additionally, new Kafka consumer configuration properties have been introduced for improved session management and polling behavior, streamlining operations across the system. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CalculatorController
participant DemandService
participant DemandGenerationConsumer
User->>CalculatorController: Request Bulk Demand
CalculatorController->>DemandService: Generate Bulk Demand
DemandService->>DemandGenerationConsumer: Push to Generation Topic
DemandGenerationConsumer-->>DemandService: Confirmation
DemandService-->>CalculatorController: Response
CalculatorController-->>User: Bulk Demand Processed
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (5)
municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/web/controller/CalculatorController.java (1)
82-82
: Add space after the colon in the log message.For better readability, add a space after the colon in the log message.
- log.info("got call to generatedemand for tenantid:"+bulkDemand.getTenantId()+" period:"+bulkDemand.getBillingPeriod()); + log.info("got call to generatedemand for tenantid: " + bulkDemand.getTenantId() + " period: " + bulkDemand.getBillingPeriod());municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/consumer/DemandGenerationConsumer.java (4)
578-579
: Add space after the colon in the log message.For better readability, add a space after the colon in the log message.
- log.info("CALL FROM TOPIC egov.generate.bulk.demand.manually.topic for tenantid:" - +bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" Start Time:"+System.currentTimeMillis() ); + log.info("CALL FROM TOPIC egov.generate.bulk.demand.manually.topic for tenantid: " + +bulkDemand.getTenantId() + " BillPeriod: " + billingPeriod + " Start Time: " + System.currentTimeMillis() );
582-583
: Add space after the colon in the log message.For better readability, add a space after the colon in the log message.
- log.info("System end time for Tenantid:"+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" end Time:"+System.currentTimeMillis()); + log.info("System end time for Tenantid: " + bulkDemand.getTenantId() + " BillPeriod: " + billingPeriod + " end Time: " + System.currentTimeMillis());
596-596
: Fix typo in the log message.Correct the typo "GOt" to "Got" and "inn" to "in" in the log message.
- log.info("GOt call inn ws-gennerate-demand-bulk topic Started time:" +System.currentTimeMillis()); + log.info("Got call in ws-generate-demand-bulk topic Started time: " + System.currentTimeMillis());
611-611
: Fix typo in the log message.Correct the typo "GOt" to "Got" and "inn" to "in" in the log message.
- log.info("GOt call inn ws-gennerate-demand-bulk topic end time:" +System.currentTimeMillis()); + log.info("Got call in ws-generate-demand-bulk topic end time: " + System.currentTimeMillis());
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/consumer/DemandGenerationConsumer.java (4 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/DemandService.java (1 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/web/controller/CalculatorController.java (1 hunks)
- municipal-services/ws-calculator/src/main/resources/application.properties (1 hunks)
Files skipped from review due to trivial changes (1)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/DemandService.java
Additional comments not posted (1)
municipal-services/ws-calculator/src/main/resources/application.properties (1)
163-165
: LGTM!The added Kafka consumer settings enhance control over session timeouts and polling intervals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/consumer/DemandGenerationConsumer.java (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/consumer/DemandGenerationConsumer.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (1)
municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/consumer/DemandGenerationConsumer.java (1)
Line range hint
589-639
:
Fix typographical error in log message.The log message at line 635 contains a typographical error ("GOt call inn"). It should be corrected to "Got call in".
- log.info("GOt call inn ws-gennerate-demand-bulk topic end time:" + System.currentTimeMillis()); + log.info("Got call in ws-generate-demand-bulk topic end time:" + System.currentTimeMillis());
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/consumer/DemandGenerationConsumer.java (5 hunks)
- municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/builder/WsQueryBuilder.java (2 hunks)
- utilities/rollout-dashboard-cronjob-indexer/app.py (4 hunks)
- utilities/rollout-dashboard-cronjob/app.py (5 hunks)
Additional comments not posted (8)
municipal-services/ws-services/src/main/java/org/egov/waterconnection/repository/builder/WsQueryBuilder.java (2)
70-86
: LGTM! The new SQL query string is well-structured.The new
WATER_PLANE_SEARCH_QUERY
string is comprehensive and enhances the information returned for water connections.
683-686
: LGTM! The method updates are appropriate.The
getSearchQueryStringForPlaneSearch
method now uses the newWATER_PLANE_SEARCH_QUERY
string, enhancing the search functionality.utilities/rollout-dashboard-cronjob/app.py (4)
20-20
: Unused environment variable.The environment variable
IFIX_DEP_ENTITY_URL
is fetched but not utilized in the function. Ensure this is intentional or remove the unused variable.
58-58
: Verify URL handling.The URL is printed but not used in any subsequent operations. Ensure this is intentional or update the code to utilize the URL appropriately.
796-797
: Verify handling of the new columntenantName
.Ensure that the new column
tenantName
is correctly handled in therecord_to_insert
tuple and the corresponding SQL insert query.
897-897
: Verify database schema update.Ensure that the new column
tenantName
is correctly added to theroll_out_dashboard
table schema.utilities/rollout-dashboard-cronjob-indexer/app.py (2)
22-22
: LGTM! Environment variable usage approved.The use of the
IFIX_DEP_ENTITY_URL
environment variable enhances flexibility and configurability.
1049-1049
: Verify the impact of removing database management logic.The removal of the database management logic simplifies the function but may affect its overall functionality. Ensure that the necessary database operations are handled elsewhere or are no longer required.
@@ -56,7 +57,7 @@ def getGPWSCHeirarchy(): | |||
if tenant.get('city') is not None and tenant.get('city').get('code') is not None: | |||
teanant_data_Map.update({tenant.get('city').get('code'): tenant.get('code')}) | |||
|
|||
url = 'https://mgramseva-dwss.punjab.gov.in/' | |||
# url = 'https://mgramseva-dwss.punjab.gov.in/' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented-out code.
The commented-out hardcoded URL should be removed for code cleanliness.
- # url = 'https://mgramseva-dwss.punjab.gov.in/'
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.
# url = 'https://mgramseva-dwss.punjab.gov.in/' |
|
||
/*if (!waterCalculatorDao.isDemandExists(tenantId, previousFromDate.getTimeInMillis(), | ||
previousToDate.getTimeInMillis(), consumerCodes) | ||
&& !waterCalculatorDao.isConnectionExists(tenantId, previousFromDate.getTimeInMillis(), | ||
previousToDate.getTimeInMillis(), consumerCodes)) { | ||
log.warn("this connection doen't have the demand in previous billing cycle :" + connectionNo); | ||
failedConnectionNos.add(connectionNo); | ||
continue; | ||
} | ||
}*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented-out code.
The commented-out code should be removed to keep the codebase clean unless there is a specific reason to retain it.
- /*if (!waterCalculatorDao.isDemandExists(tenantId, previousFromDate.getTimeInMillis(),
- previousToDate.getTimeInMillis(), consumerCodes)
- && !waterCalculatorDao.isConnectionExists(tenantId, previousFromDate.getTimeInMillis(),
- previousToDate.getTimeInMillis(), consumerCodes)) {
- log.warn("this connection doen't have the demand in previous billing cycle :" + connectionNo);
- failedConnectionNos.add(connectionNo);
- continue;
- }*/
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.
/*if (!waterCalculatorDao.isDemandExists(tenantId, previousFromDate.getTimeInMillis(), | |
previousToDate.getTimeInMillis(), consumerCodes) | |
&& !waterCalculatorDao.isConnectionExists(tenantId, previousFromDate.getTimeInMillis(), | |
previousToDate.getTimeInMillis(), consumerCodes)) { | |
log.warn("this connection doen't have the demand in previous billing cycle :" + connectionNo); | |
failedConnectionNos.add(connectionNo); | |
continue; | |
} | |
}*/ |
log.info("CALL FROM TOPIC egov.generate.bulk.demand.manually.topic for tenantid:" | ||
+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" Start Time:"+System.currentTimeMillis() ); | ||
Long starTime = System.currentTimeMillis(); | ||
generateDemandAndSendnotification(bulkDemand.getRequestInfo(), bulkDemand.getTenantId(), billingPeriod, billingMasterData, | ||
isSendMessage, isManual); | ||
long endTime=System.currentTimeMillis(); | ||
long diff = endTime-starTime; | ||
log.info("time takenn to generate demand for Tenantid:"+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" is : "+diff/1000 +" secs"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typographical error in log message.
The log message at line 575 contains a typographical error ("time takenn"). It should be corrected to "time taken".
- log.info("time takenn to generate demand for Tenantid:"+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" is : "+diff/1000 +" secs");
+ log.info("time taken to generate demand for Tenantid:"+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" is : "+diff/1000 +" secs");
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.
log.info("CALL FROM TOPIC egov.generate.bulk.demand.manually.topic for tenantid:" | |
+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" Start Time:"+System.currentTimeMillis() ); | |
Long starTime = System.currentTimeMillis(); | |
generateDemandAndSendnotification(bulkDemand.getRequestInfo(), bulkDemand.getTenantId(), billingPeriod, billingMasterData, | |
isSendMessage, isManual); | |
long endTime=System.currentTimeMillis(); | |
long diff = endTime-starTime; | |
log.info("time takenn to generate demand for Tenantid:"+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" is : "+diff/1000 +" secs"); | |
log.info("CALL FROM TOPIC egov.generate.bulk.demand.manually.topic for tenantid:" | |
+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" Start Time:"+System.currentTimeMillis() ); | |
Long starTime = System.currentTimeMillis(); | |
generateDemandAndSendnotification(bulkDemand.getRequestInfo(), bulkDemand.getTenantId(), billingPeriod, billingMasterData, | |
isSendMessage, isManual); | |
long endTime=System.currentTimeMillis(); | |
long diff = endTime-starTime; | |
log.info("time taken to generate demand for Tenantid:"+bulkDemand.getTenantId()+" BillPeriod:"+billingPeriod+" is : "+diff/1000 +" secs"); |
Set<String> consumerCodes = new LinkedHashSet<String>(); | ||
consumerCodes.add(calculationReq.getCalculationCriteria().get(0).getConnectionNo()); | ||
if (!waterCalculatorDao.isDemandExists(tenantId, previousFromDate.getTimeInMillis(), | ||
previousToDate.getTimeInMillis(), consumerCodes) | ||
&& !waterCalculatorDao.isConnectionExists(tenantId, previousFromDate.getTimeInMillis(), | ||
previousToDate.getTimeInMillis(), consumerCodes)) { | ||
log.warn("this connection doen't have the demand in previous billing cycle :" + calculationReq.getCalculationCriteria().get(0).getConnectionNo()); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typographical error in log message.
The log message at line 632 contains a typographical error ("doen't"). It should be corrected to "doesn't".
- log.warn("this connection doen't have the demand in previous billing cycle :" + calculationReq.getCalculationCriteria().get(0).getConnectionNo());
+ log.warn("this connection doesn't have the demand in previous billing cycle :" + calculationReq.getCalculationCriteria().get(0).getConnectionNo());
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.
Set<String> consumerCodes = new LinkedHashSet<String>(); | |
consumerCodes.add(calculationReq.getCalculationCriteria().get(0).getConnectionNo()); | |
if (!waterCalculatorDao.isDemandExists(tenantId, previousFromDate.getTimeInMillis(), | |
previousToDate.getTimeInMillis(), consumerCodes) | |
&& !waterCalculatorDao.isConnectionExists(tenantId, previousFromDate.getTimeInMillis(), | |
previousToDate.getTimeInMillis(), consumerCodes)) { | |
log.warn("this connection doen't have the demand in previous billing cycle :" + calculationReq.getCalculationCriteria().get(0).getConnectionNo()); | |
} else { | |
Set<String> consumerCodes = new LinkedHashSet<String>(); | |
consumerCodes.add(calculationReq.getCalculationCriteria().get(0).getConnectionNo()); | |
if (!waterCalculatorDao.isDemandExists(tenantId, previousFromDate.getTimeInMillis(), | |
previousToDate.getTimeInMillis(), consumerCodes) | |
&& !waterCalculatorDao.isConnectionExists(tenantId, previousFromDate.getTimeInMillis(), | |
previousToDate.getTimeInMillis(), consumerCodes)) { | |
log.warn("this connection doesn't have the demand in previous billing cycle :" + calculationReq.getCalculationCriteria().get(0).getConnectionNo()); | |
} else { |
Summary by CodeRabbit
New Features
Bug Fixes
Chores