fix(sql): Bugfixes for sql matching stats - #3915
Conversation
7d52893 to
d679125
Compare
| } | ||
| catch (SQLException exception) { | ||
| log.error("FAILED connecting to {}", connection.getJdbcConnectionUrl(), exception); | ||
| throw exception; |
There was a problem hiding this comment.
Dadurch muss beim Startup die Datenbank schon verfügbar sein und kann nicht erst verzögert bereit sein. Ist für kubernetes okay, da der Pod dann neugestartet werden kann, aber du könntest auch die Exception mit der neuen Message wrappen als Cause un dann würde der Exception nicht doppelt geloggt werden (Ich gehe davon aus, dass die Exception dann nochmal geloggt wird )
There was a problem hiding this comment.
Für mich ist das schon Anforderung der Anwendung, dass SQL verfügbar ist, wenn es startet? Siehst du das anders? Mir gehts auch darum, dass ein falsches Passwort den Start abbricht, statt erst später beim Anfrage ausführen (also ggf. gar nicht) knallt
There was a problem hiding this comment.
Mir ging es hier darum, dass die Exception vermutlich doppelt geloggt wird.
Einmal hier und dann durch den rethrow nochmals an anderer Stelle sodass man in den Logs zweimal fast den selben Stacktrace hat, was verwirrend ist. Deshalb die Exception wrappen mit einer (z.B.) RuntimeException wrappen, die aber als Message die momentane Log-Message hat
| /** | ||
| * collect unique fields used/defined in the expressions. | ||
| */ | ||
| private static List<Field<?>> collectAllFields(List<CTCondition.ConceptConditions> conceptConditions) { |
There was a problem hiding this comment.
| /** | |
| * collect unique fields used/defined in the expressions. | |
| */ | |
| private static List<Field<?>> collectAllFields(List<CTCondition.ConceptConditions> conceptConditions) { | |
| /** | |
| * collect unique fields used/defined in the expressions. | |
| */ | |
| private static List<Field<?>> collectReferencedFields(List<CTCondition.ConceptConditions> conceptConditions) { |
|
|
||
| Stopwatch stopwatch = Stopwatch.createStarted(); | ||
| MatchingStats stats = new MatchingStats(); | ||
| stats.putEntry("sql", entry.getValue()); |
There was a problem hiding this comment.
Dass der Entry hier "sql" heißt, sollte das nicht aus einer Konstante kommen?
| DialectBundle dialectBundle, | ||
| DSLContext dslContext, | ||
| SqlExecutionService executionService, Clock clock | ||
| SqlExecutionService executionService, Clock clock, String defaultPrimaryColumn |
There was a problem hiding this comment.
| SqlExecutionService executionService, Clock clock, String defaultPrimaryColumn | |
| SqlExecutionService executionService, | |
| Clock clock, | |
| String defaultPrimaryColumn |
awildturtok
changed the title
| } | ||
| catch (SQLException exception) { | ||
| log.error("FAILED connecting to {}", connection.getJdbcConnectionUrl(), exception); | ||
| throw exception; |
There was a problem hiding this comment.
Mir ging es hier darum, dass die Exception vermutlich doppelt geloggt wird.
Einmal hier und dann durch den rethrow nochmals an anderer Stelle sodass man in den Logs zweimal fast den selben Stacktrace hat, was verwirrend ist. Deshalb die Exception wrappen mit einer (z.B.) RuntimeException wrappen, die aber als Message die momentane Log-Message hat
| for (Map.Entry<ConceptId, ListenableFuture<?>> conceptState : jobsByConcept.entrySet()) { | ||
| if (conceptState.getValue().state().equals(Future.State.FAILED)) { | ||
| log.warn("FAILED to collect SQL matching stats for {}", conceptState.getKey(), conceptState.getValue().exceptionNow()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Da du schon ListenableFutures benutzt und das logging keine Seiteneffekte hat, kannst du auch einfach einen Callback registrieren und muss hier nicht loopen
| try { | ||
| log.debug("TEST connecting to {}", connection.getJdbcConnectionUrl()); | ||
| if (dataSource.getConnection().isValid(100)) { | ||
| if (dataSource.getConnection().isValid(1000)) { |
There was a problem hiding this comment.
Die 1000 ist ein Timeout oder?
There was a problem hiding this comment.
genau, der geht hier aber nur auf die connection. Denke, wenn die Verbindung nach 1sek nicht als valide aufgebaut werden kann ist da was kaputt
2 participants
No description provided.