Fix/36265 pa fsm #1229
Fix/36265 pa fsm #1229
Conversation
marregui
force-pushed
the
fix/36265-pa-fsm
branch
from
cf61378 to
5c1bd96
Compare
…esources after unit test completes
marregui
force-pushed
the
fix/36265-pa-fsm
branch
from
4ff7b05 to
d3ba8de
Compare
| } | ||
| return errorResponse(new AdapterFailedSchemaValidationError(errorMessages.toErrorList())); | ||
| } | ||
| }).orElseGet(adapterNotFoundError(adapterId)); |
There was a problem hiding this comment.
I suggest adapterNotFoundError(adapterId) to be in a function.
| log.error("Failed to restart adapter '{}'.", adapterId, throwable); | ||
| } else { | ||
| log.trace("Adapter '{}' was restarted successfully.", adapterId); | ||
| log.info("Adapter '{}' was restarted successfully.", adapterId); |
There was a problem hiding this comment.
That actually has an information character.
I would suggest keeping it at info.
These operations are very low frequence.
There was a problem hiding this comment.
The reason I asked this question is the other similar logging statements are all trace ones, but this one is info.
hivemq-edge/src/main/java/com/hivemq/fsm/ProtocolAdapterFSM.java
Outdated
Show resolved
Hide resolved
hivemq-edge/src/main/java/com/hivemq/fsm/ProtocolAdapterFSM.java
Outdated
Show resolved
Hide resolved
hivemq-edge/src/main/java/com/hivemq/fsm/ProtocolAdapterFSM.java
Outdated
Show resolved
Hide resolved
| try { | ||
| subscription.delete(); | ||
| } catch (final Exception e) { | ||
| log.warn("Failed to delete subscription {}: {}", subscription, e.getMessage()); |
| try { | ||
| client.removeSubscription(subscription); | ||
| } catch (final Exception e) { | ||
| log.warn("Failed to remove subscription {}: {}", subscription, e.getMessage()); |
| try { | ||
| client.removeFaultListener(faultListener); | ||
| } catch (final Throwable e) { | ||
| log.error("Failed to remove fault listener {}: {}", faultListener, e.getMessage()); |
| try { | ||
| client.removeSessionActivityListener(activityListener); | ||
| } catch (final Throwable e) { | ||
| log.error("Failed to remove session activity listener {}: {}", activityListener, e.getMessage()); |
| client.disconnect(); | ||
| log.info("Successfully disconnected OPC UA client"); | ||
| } catch (final UaException e) { | ||
| log.error("Failed to disconnect: {}", e.getMessage()); |
| } catch (final Exception e) { | ||
| log.error("Error closing HikariCP datasource", e); | ||
| } finally { | ||
| ds = null; // Clear reference to allow GC |
There was a problem hiding this comment.
Same suggestion. ds = null; should be executed among all branches.
… hot reload for mappings and tags (cheaper). will see how this improves stability.
| StateEnum.ERROR_CLOSING, Set.of(StateEnum.ERROR), | ||
| StateEnum.ERROR, Set.of(StateEnum.CONNECTING, StateEnum.DISCONNECTED), // can recover from error | ||
| StateEnum.CLOSED, Set.of(StateEnum.DISCONNECTED, StateEnum.CLOSING) // can restart from closed or go to closing | ||
| public static final @NotNull Map<StateEnum, Set<StateEnum>> possibleTransitions = Map.of(StateEnum.DISCONNECTED, |
There was a problem hiding this comment.
I suppose java.util.function.Function<StateEnum, StateEnum> with switch-case inside can outperform this Map.
| * @param tags the new tags | ||
| */ | ||
| public void setTags(final @NotNull List<? extends Tag> tags) { | ||
| this.tags = tags; |
There was a problem hiding this comment.
Are there locking mechanism for protecting these lists? Is it possible the multiple adapters are accessing these lists in parallel?
| .whenComplete((result, throwable) -> { | ||
| if (throwable != null) { | ||
| log.error("Error starting adapter", throwable); | ||
| stopProtocolAdapterOnFailedStart(); |
There was a problem hiding this comment.
During this call stopProtocolAdapterOnFailedStart(), currentStartFuture is not null. Is that the expected behavior?
Also if the exception is from the previous throw new RuntimeException("Failed to start consumers", startException);, stopProtocolAdapterOnFailedStart() will be called twice. Is that the expected behavior?
| // Transition southbound state to indicate shutdown in progress | ||
| final StateEnum currentSouthboundState = currentState().southbound(); | ||
| if (currentSouthboundState == StateEnum.CONNECTED) { | ||
| transitionSouthboundState(StateEnum.DISCONNECTING); |
3 participants
card: https://hivemq.kanbanize.com/ctrl_board/57/cards/37501/details/