added connection and retry options for jsm client #408

astelmashenko · 2023-06-29T13:26:26Z

Proposed Changes

🎁 Add new feature

The PR introduces reconnect options, connection handler to log in case of connection lost or reconnect attempts finished. In case it could not reconnect it halts dispatcher, so k8s can restart it.

Release Note

Add additional connection options:
config-nats ConfigMap supports the next parameters: connOpts.retryOnFailedConnect, maxReconnects, reconnectWait

see docs/jetstream.md for details

codecov · 2023-06-29T13:32:06Z

Codecov Report

Merging #408 (da7c0b1) into main (030b206) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #408   +/-   ##
=======================================
  Coverage   38.97%   38.97%           
=======================================
  Files          29       29           
  Lines        1919     1919           
=======================================
  Hits          748      748           
  Misses       1125     1125           
  Partials       46       46

astelmashenko · 2023-06-29T13:46:48Z

/assign @dan-j

knative-prow · 2023-06-29T13:46:50Z

@astelmashenko: GitHub didn't allow me to assign the following users: dan-j.

Note that only knative-sandbox members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @dan-j

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

astelmashenko · 2023-06-30T09:47:08Z

@matzew , work is complete, ready for review. Are you going to review or should I assign somebody else (before I worked with Evan Anderson and Lionel Villard)?
Thanks

astelmashenko · 2023-06-30T09:47:29Z

cc @dan-j

astelmashenko · 2023-06-30T13:13:37Z

/assign @matzew

astelmashenko · 2023-07-03T16:21:41Z

/assign @lionelvillard

dan-j · 2023-07-04T10:57:10Z

Apologies, just caught up on notifications and seen this. Changes seem sensible, thanks for the contribution!

I'm not sure I have permissions to lgtm/approve but trying anyway

/lgtm
/approve

knative-prow · 2023-07-04T10:57:12Z

@dan-j: changing LGTM is restricted to collaborators

In response to this:

Apologies, just caught up on notifications and seen this. Changes seem sensible, thanks for the contribution!

I'm not sure I have permissions to lgtm/approve but trying anyway

/lgtm
/approve

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

astelmashenko · 2023-07-04T11:27:05Z

@evankanderson , could you please review and put lgtm label?

astelmashenko · 2023-07-06T07:19:41Z

/lgtm

knative-prow · 2023-07-06T07:19:42Z

@astelmashenko: you cannot LGTM your own PR.

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

pierDipi · 2023-07-07T15:00:45Z

pkg/common/nats/conn.go

+	// reconnection options
+	if config.ConnOpts.RetryOnFailedConnect {


I think you need to check for config.ConnOpts != nil

pierDipi · 2023-07-07T15:03:20Z

pkg/common/config/eventingnatsconfig.go

+	// ReconnectWait time between reconnects in milliseconds
+	ReconnectWait time.Duration `json:"reconnectWait,omitempty"`


Usually, Kubernetes recommends to have the unit in the name: https://github.com/zecke/Kubernetes/blob/master/docs/devel/api-conventions.md#units and the type being just int the reason being, time.Duration is a Go only type and format, so it limits the client that can easily use the API

pierDipi · 2023-07-07T15:04:10Z

pkg/common/nats/conn.go

+			if attempts < 5 {
+				logger.Infof("Attempts left: %d", config.ConnOpts.MaxReconnects-attempts)
+			}


Why is 5 hardcoded here?

This is just to show 5 last messages of recconnecting attepmts. Should I give it a name, make it debug and log everything or just remove it?

pierDipi · 2023-07-07T15:06:43Z

@astelmashenko @dan-j Regarding lgtm and approve permissions, we can give you them if there is interest, also because you're the one maintaining the Eventing with Nats integration, would you be interested in that?

…d magic number and changed log to be debug

astelmashenko · 2023-07-11T13:32:17Z

@astelmashenko @dan-j Regarding lgtm and approve permissions, we can give you them if there is interest, also because you're the one maintaining the Eventing with Nats integration, would you be interested in that?

I'm already able to lgtm/approve. Dan-j was going to apply for that as well.

astelmashenko · 2023-07-11T13:32:43Z

@pierDipi , could you please take a look. I've addressed all the comments.

pierDipi · 2023-07-11T14:54:20Z

/lgtm

pierDipi · 2023-07-11T14:54:25Z

/approve

knative-prow · 2023-07-11T14:54:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astelmashenko, dan-j, pierDipi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [pierDipi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

astelmashenko · 2024-01-12T09:25:26Z

/cherrypick main

knative-prow-robot · 2024-01-12T09:25:29Z

@astelmashenko: base branch (main) needs to differ from target branch (main)

In response to this:

/cherrypick main

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

astelmashenko added 2 commits June 29, 2023 14:31

added connection and retry options for jsm client

56adb8e

updated readme

e98078c

knative-prow bot requested review from matzew and odacremolbap June 29, 2023 13:26

knative-prow bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 29, 2023

go imports for files

f8c49ca

fixed typos

a867884

reverted controller, because it does not have relation to PR

c8db783

knative-prow bot assigned matzew Jun 30, 2023

knative-prow bot assigned lionelvillard Jul 3, 2023

pierDipi reviewed Jul 7, 2023

View reviewed changes

changed reconnect wait to int, changed name to have time unit, remove…

da7c0b1

…d magic number and changed log to be debug

astelmashenko mentioned this pull request Jul 7, 2023

implemented reconnects for jetstream channels #409

Merged

astelmashenko requested a review from pierDipi July 10, 2023 06:28

knative-prow bot assigned pierDipi Jul 11, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2023

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2023

knative-prow bot merged commit 4b73aaa into knative-extensions:main Jul 11, 2023

astelmashenko deleted the feature/jsm-connection-options branch July 12, 2023 06:39

dan-j mentioned this pull request Oct 19, 2023

NATS reconnect jitter incorrect #445

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added connection and retry options for jsm client #408

added connection and retry options for jsm client #408

astelmashenko commented Jun 29, 2023

codecov bot commented Jun 29, 2023 •

edited

Loading

astelmashenko commented Jun 29, 2023

knative-prow bot commented Jun 29, 2023

astelmashenko commented Jun 30, 2023

astelmashenko commented Jun 30, 2023

astelmashenko commented Jun 30, 2023

astelmashenko commented Jul 3, 2023

dan-j commented Jul 4, 2023

knative-prow bot commented Jul 4, 2023

astelmashenko commented Jul 4, 2023

astelmashenko commented Jul 6, 2023

knative-prow bot commented Jul 6, 2023

pierDipi Jul 7, 2023

astelmashenko Jul 7, 2023

pierDipi Jul 7, 2023

astelmashenko Jul 7, 2023

pierDipi Jul 7, 2023

astelmashenko Jul 7, 2023 •

edited

Loading

astelmashenko Jul 7, 2023

pierDipi commented Jul 7, 2023

astelmashenko commented Jul 11, 2023

astelmashenko commented Jul 11, 2023

pierDipi commented Jul 11, 2023

pierDipi commented Jul 11, 2023

knative-prow bot commented Jul 11, 2023

astelmashenko commented Jan 12, 2024

knative-prow-robot commented Jan 12, 2024

		// reconnection options
		if config.ConnOpts.RetryOnFailedConnect {

		// ReconnectWait time between reconnects in milliseconds
		ReconnectWait time.Duration `json:"reconnectWait,omitempty"`

added connection and retry options for jsm client #408

added connection and retry options for jsm client #408

Conversation

astelmashenko commented Jun 29, 2023

Proposed Changes

codecov bot commented Jun 29, 2023 • edited Loading

Codecov Report

astelmashenko commented Jun 29, 2023

knative-prow bot commented Jun 29, 2023

astelmashenko commented Jun 30, 2023

astelmashenko commented Jun 30, 2023

astelmashenko commented Jun 30, 2023

astelmashenko commented Jul 3, 2023

dan-j commented Jul 4, 2023

knative-prow bot commented Jul 4, 2023

astelmashenko commented Jul 4, 2023

astelmashenko commented Jul 6, 2023

knative-prow bot commented Jul 6, 2023

pierDipi Jul 7, 2023

Choose a reason for hiding this comment

astelmashenko Jul 7, 2023

Choose a reason for hiding this comment

pierDipi Jul 7, 2023

Choose a reason for hiding this comment

astelmashenko Jul 7, 2023

Choose a reason for hiding this comment

pierDipi Jul 7, 2023

Choose a reason for hiding this comment

astelmashenko Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

astelmashenko Jul 7, 2023

Choose a reason for hiding this comment

pierDipi commented Jul 7, 2023

astelmashenko commented Jul 11, 2023

astelmashenko commented Jul 11, 2023

pierDipi commented Jul 11, 2023

pierDipi commented Jul 11, 2023

knative-prow bot commented Jul 11, 2023

astelmashenko commented Jan 12, 2024

knative-prow-robot commented Jan 12, 2024

codecov bot commented Jun 29, 2023 •

edited

Loading

astelmashenko Jul 7, 2023 •

edited

Loading