Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LibraryTimeout publish sessions #519

Merged
merged 1 commit into from
Sep 3, 2024
Merged

LibraryTimeout publish sessions #519

merged 1 commit into from
Sep 3, 2024

Conversation

devesh-b1
Copy link
Contributor

@devesh-b1 devesh-b1 commented Aug 29, 2024 •
edited
Loading

Changes to expose Session info (LibraryTimeOut) owned by that timed out library instance, because "engine.libraries().resultIfPresent().get(0).sessions()" does not contain library info once it was timed out (stopped)

@devesh-b1 devesh-b1 force-pushed the LibraryTimeout-publish-sessions branch 2 times, most recently from 0a6653f to 8adb98e Compare August 29, 2024 14:03
wojciech-adaptive
wojciech-adaptive requested changes Sep 3, 2024
View reviewed changes
Copy link
Collaborator

@wojciech-adaptive wojciech-adaptive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to expose that message on libraries? Otherwise it's not very useful.

artio-codecs/src/main/resources/uk/co/real_logic/artio/messages/message-schema.xml Outdated
@@ -499,6 +499,9 @@
description="notifies library instances that they have been timed out, added for monitoring purposes">
<field name="libraryId" id="1" type="LibraryId"/>
<field name="connectCorrelationId" id="2" type="CorrelationId"/>
<group name="sessions" id="76" dimensionType="groupSizeEncoding">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should increment the version and declare since which this group is present. Also any particular reason for those IDs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Thanks.

artio-core/src/main/java/uk/co/real_logic/artio/protocol/GatewayPublication.java Outdated
@@ -125,6 +127,8 @@ public class GatewayPublication extends ClaimablePublication
private static final int THROTTLE_CONFIGURATION_REPLY_LENGTH = HEADER_LENGTH +
ThrottleConfigurationReplyEncoder.BLOCK_LENGTH;
private static final int SEQ_INDEX_SYNC_LENGTH = HEADER_LENGTH + SeqIndexSyncEncoder.BLOCK_LENGTH;
private static final int LIBRARY_TIMEOUT_LENGTH = HEADER_LENGTH + LibraryTimeoutEncoder.BLOCK_LENGTH +
GroupSizeEncodingEncoder.ENCODED_LENGTH * 2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why * 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry not needed for this one. Removed. Thanks.

artio-core/src/test/java/uk/co/real_logic/artio/engine/framer/FramerTest.java Outdated
@@ -140,6 +141,12 @@ public class FramerTest

private final MutableLong connectionId = new MutableLong(NO_CONNECTION_ID);
private final ErrorHandler errorHandler = mock(ErrorHandler.class);
LivenessDetector livenessDetector = mock(LivenessDetector.class);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private final

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Thanks.

@devesh-b1 devesh-b1 force-pushed the LibraryTimeout-publish-sessions branch from 8adb98e to 876f9cf Compare September 3, 2024 09:57
@devesh-b1
Copy link
Contributor Author

Do you want to expose that message on libraries? Otherwise it's not very useful.

@wojciech-adaptive
We use this on Engine side to extract the sessionIds for crashed library and trigger disconnect session and reset sequence numbers using DisconnectSessionRequestEncoder and AdminResetSequenceNumbersRequestEncoder (AdminEngineProtocolSubscription)
Thanks.

wojciech-adaptive
wojciech-adaptive requested changes Sep 3, 2024
View reviewed changes
artio-codecs/src/main/resources/uk/co/real_logic/artio/messages/message-schema.xml Outdated
@@ -499,6 +499,9 @@
description="notifies library instances that they have been timed out, added for monitoring purposes">
<field name="libraryId" id="1" type="LibraryId"/>
<field name="connectCorrelationId" id="2" type="CorrelationId"/>
<group name="sessions" id="76" dimensionType="groupSizeEncoding" sinceVersion="25">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since 26. The IDs should be 3 and 4.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Thanks.

@wojciech-adaptive
Copy link
Collaborator

Do you want to expose that message on libraries? Otherwise it's not very useful.

@wojciech-adaptive We use this on Engine side to extract the sessionIds for crashed library and trigger disconnect session and reset sequence numbers using DisconnectSessionRequestEncoder and AdminResetSequenceNumbersRequestEncoder (AdminEngineProtocolSubscription) Thanks.

Do you have some custom thread/process subscribed to those messages?

@devesh-b1 devesh-b1 force-pushed the LibraryTimeout-publish-sessions branch from 876f9cf to 3654cf2 Compare September 3, 2024 10:55
@devesh-b1
Copy link
Contributor Author

Do you want to expose that message on libraries? Otherwise it's not very useful.

@wojciech-adaptive We use this on Engine side to extract the sessionIds for crashed library and trigger disconnect session and reset sequence numbers using DisconnectSessionRequestEncoder and AdminResetSequenceNumbersRequestEncoder (AdminEngineProtocolSubscription) Thanks.

Do you have some custom thread/process subscribed to those messages?
Hi @wojciech-adaptive
Thanks.
Yes we have custom polling thread listening to LibraryTimeout message
Once received, we publish Disconnect and Reset Sequence message on stream id 22 using:
"final ExclusivePublication publication = aeron.addExclusivePublication(channel, 22);"
Stream id 22 is ArtioAdminConfiguration.DEFAULT_OUTBOUND_ADMIN_STREAM_ID

@wojciech-adaptive wojciech-adaptive merged commit 3171046 into real-logic:master Sep 3, 2024
6 checks passed
grafstrom pushed a commit to VermicFinTech/artio that referenced this pull request Nov 20, 2024
@devesh-b1 @grafstrom
LibraryTimeout publish sessions (real-logic#519)
95053cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wojciech-adaptive wojciech-adaptive wojciech-adaptive requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@devesh-b1 @wojciech-adaptive