Possibility to reset connection to redis#41
Possibility to reset connection to redis#41vdoleans01 merged 13 commits intoAmadeusITGroup:masterfrom
Conversation
mgarcez
left a comment
mgarcez
left a comment
There was a problem hiding this comment.
Hi Vincent,
Thank you for the PR.
I added some comments.
Could you please also perform a formatting of your code with your IDE? And also remove unnecessary newlines new added?
Thanks,
Maxime
| redis.info("server"); | ||
| return true; | ||
| } | ||
| catch ( Exception e ) { |
There was a problem hiding this comment.
Are there no other ways to detect that Redis is disconnected than by catching an exception?
I would rather we find another way. If not possible, then can we at least be more specific on the Exception caught?
| * max: if the number if items is more than the max value the ErrorTracker reachLimits | ||
| * | ||
| */ | ||
|
|
There was a problem hiding this comment.
Better to remove the useless end of line :)
| import java.util.concurrent.ConcurrentLinkedQueue; | ||
|
|
||
| /** | ||
| * The class is used to track the number of item during a time. |
|
|
||
| /** | ||
| * The class is used to track the number of item during a time. | ||
| * input parameter |
| private ConcurrentLinkedQueue<Long> list = new ConcurrentLinkedQueue<Long>(); | ||
|
|
||
| /** | ||
| * The class calculate the items during a time |
There was a problem hiding this comment.
calculates
Could you clarify "during a time"?
|
|
||
| monitoring = new MetricRegistry (); | ||
|
|
||
| String SESSIONS_METRIC_MANAGER_PREFIX = "com.amadeus.session.manager"; |
There was a problem hiding this comment.
You could put is as a constant
| try { | ||
| expirationListener.close(redis); | ||
| expirationListener = null; | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
Cannot we be more specific on the Exception caught?
Same remark for the other cases where we caught Exception
Small spelling error: generates errors
There was a problem hiding this comment.
No we cant , it is like you want to close a sql connection you want to close is even you have other errors
| */ | ||
| final int max ; | ||
|
|
||
| public void addError( long now ) { |
There was a problem hiding this comment.
Maybe you could add some javadoc explaining the method?
| monitoring = new MetricRegistry (); | ||
|
|
||
| String SESSIONS_METRIC_MANAGER_PREFIX = "com.amadeus.session.manager"; | ||
| String ResetManager_created_METRIC = name(SESSIONS_METRIC_MANAGER_PREFIX, "initialized"); |
There was a problem hiding this comment.
Would put it, and the others as constants (and hence all in upper case too)
| private final ErrorTracker errorTracker ; | ||
|
|
||
|
|
||
| private final Meter create_metrics ; |
There was a problem hiding this comment.
Would either put the variable name as camelCase, or as a CONSTANT_NAME.
I would also replace "metrics" by "meter", or "metric" in singular.
2 participants
No description provided.