track cache usage #69
Conversation
| @@ -263,7 +263,7 @@ void translationPangeanicNoSrcMultipleLanguages() throws Exception { | |||
| .accept(MediaType.APPLICATION_JSON) | |||
| .contentType(MediaType.APPLICATION_JSON) | |||
| .content(requestJson)) | |||
| .andExpect(status().isOk()) | |||
| // .andExpect(status().isOk()) | |||
There was a problem hiding this comment.
any reason why this was commented out?
There was a problem hiding this comment.
Removed, it works fine.
| @@ -39,7 +39,7 @@ | |||
| public abstract class BaseTranslationTest extends IntegrationTestUtils { | |||
|
|
|||
| protected MockMvc mockMvc; | |||
| protected final static int redisPort=6370; | |||
| protected final static int redisPort=16370; | |||
There was a problem hiding this comment.
we should add redis test container to integration tests within the scope of this ticket (similar to mongo container is set api) https://testcontainers.com/modules/redis/
| if(isCachingEnabled()) { | ||
| //save result in the redis cache | ||
| redisCacheService.store(toTranslate); | ||
| //logging the number of translated/cached lines and chars |
There was a problem hiding this comment.
please move the logging part to own method and use a java object to collect the metrics, e.g. TranslationCashingStats.
| int numCharsCached=translationObjs.stream().filter(el -> el.isRetrievedFromCache()).map(el -> el.getTranslation().length()).reduce(0, Integer::sum); | ||
| int numLinesTranslated=toTranslate.size(); | ||
| int numCharsTranslated=toTranslate.stream().map(el -> el.getText().length()).reduce(0, Integer::sum); | ||
| if(logger.isInfoEnabled()) { |
There was a problem hiding this comment.
if the metrics are not logs, their computation is redundand, please update acordingly.
| int numLinesCached=(int) translationObjs.stream().filter(el -> el.isRetrievedFromCache()).count(); | ||
| int numCharsCached=translationObjs.stream().filter(el -> el.isRetrievedFromCache()).map(el -> el.getTranslation().length()).reduce(0, Integer::sum); | ||
| int numLinesTranslated=toTranslate.size(); | ||
| int numCharsTranslated=toTranslate.stream().map(el -> el.getText().length()).reduce(0, Integer::sum); |
There was a problem hiding this comment.
this is incorectly computed, as it must not include the texts discarded for translation (i.e. nonTranslatable & same language)
| redisCacheService.store(toTranslate); | ||
| //logging the number of translated/cached lines and chars | ||
| int numLinesCached=(int) translationObjs.stream().filter(el -> el.isRetrievedFromCache()).count(); | ||
| int numCharsCached=translationObjs.stream().filter(el -> el.isRetrievedFromCache()).map(el -> el.getTranslation().length()).reduce(0, Integer::sum); |
There was a problem hiding this comment.
for performance reasons it is better to use a regular for loop, in which both the lines and the chars are computed.
There was a problem hiding this comment.
the computation of the lines and chars server by the translation service must be computed as well,
see last row in the comment: https://europeana.atlassian.net/browse/EA-3969?focusedCommentId=133237
No description provided.