Redis and Minor Optimization #54
Conversation
| @@ -68,7 +68,7 @@ public Response registerEndpoint(@NotNull EndpointRequest registerEndpointReques | |||
| try { | |||
| log.info(String.format("Processing register endpoint for %s", registerEndpointRequest)); | |||
|
|
|||
| Integer endpointId = clientRegisterationService.registerEndpoint(registerEndpointRequest); | |||
| String endpointId = clientRegisterationService.registerEndpoint(registerEndpointRequest); | |||
There was a problem hiding this comment.
If converting to string, then use UUID, and change .sql file as well.
| @@ -78,7 +84,7 @@ public Graph getGraphJSON(String clientName, String endpointName) throws Excepti | |||
| int clientId = clientDetails.getClientId(); | |||
|
|
|||
|
|
|||
| EndpointDetails endpointDetails = registerationDao.getEndpointDetails(clientId, endpointName); | |||
There was a problem hiding this comment.
+1 for moving this to DocumentDB from MySQL.
| @@ -46,14 +45,15 @@ | |||
|
|
|||
| // validate. | |||
| // Get ClientId , and EndpointId. | |||
| ClientDetails clientDetails = registerationDao.getClientDetails(executeWorkflowRequest.getClientName()); | |||
| ClientDetails clientDetails = executeWorkflowServiceHelper.getClientDetailsAndSave(executeWorkflowRequest.getClientName()); | |||
| @@ -36,6 +40,15 @@ | |||
| @Inject | |||
| private RabbitMQOperation rabbitMQOperation; | |||
|
|
|||
| @Inject | |||
| private RedisCache redisCache; | |||
There was a problem hiding this comment.
Ideally any caching service should not be composite in any class. It will create problem, if we want to move away from Redis in future.
Its always recommended to put all caching logic in one class, and inject that class in required classes.
We can take it later, but just mark it as TODO.
| * @return EndpointDetails | ||
| */ | ||
| public ClientDetails getClientDetailsAndSave(String clientName) { | ||
| String cacheKey = clientName; |
| dbObjectInput.append(ExecutionDocumentConstants.Fields.ENDPOINT_NAME, endpointDetails.getEndpointName()); | ||
| dbObjectInput.append(ExecutionDocumentConstants.Fields.ENDPOINT_DETAILS, JSONLoader.stringify(endpointDetails)); | ||
| collection.insertOne(dbObjectInput); | ||
| log.debug(String.format("EndPoint details are inserted : %s inserted successfully in \n collection : %s and db : %s", endpointDetails, "graph_endpoint", "shepherd_graph_endpoint")); |
There was a problem hiding this comment.
Remove all the hardcoded strings. Please make this habit from day#1. Use root level constants file for the same.
| } | ||
| } | ||
| } catch (MongoWriteException e) { | ||
| log.error(e.getMessage(), e); |
There was a problem hiding this comment.
improve logging. please use logging written as reference. Try to use single standard throughtout the service.
| @@ -36,7 +36,7 @@ public static Connection createQueueConnection(String userName, String password, | |||
| virtualHost, hostName, portNumber); | |||
| log.info(String.format("created a new Connection Id %s on %s:%d/%s",consumerConnection.getId(),hostName,portNumber,virtualHost)); | |||
| }else{ | |||
| log.warn("Connection is already established, No new connection created"); | |||
| log.warn("Connection is already established, Using already present consumer connection"); | |||
| @@ -172,11 +172,11 @@ public void handleDelivery(String consumerTag, | |||
| log.debug(String.format("Message received : %s. delivery Tag : %s.", message, deliveryTag)); | |||
| } catch (Exception e) { | |||
| log.error("Node-message processing failed.", e); | |||
| // TODO : NACK | |||
| channel.basicNack(deliveryTag,false,true); | |||
There was a problem hiding this comment.
Did you tested this NAck ? Is it pushing back to queue ? Also, we need to check feature like visibility timeout in rabbit. Reference : https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-visibility-timeout.html
If there is no such feature in Rabbit, then it may harm Shepherd threads. We need to come with some approach for the same.
| import java.security.NoSuchAlgorithmException; | ||
| import java.util.concurrent.TimeoutException; | ||
|
|
||
| public class RedisCacheTest { |
No description provided.