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

fix: Increase request timeout #102

Closed
wants to merge 1 commit into from
Closed

fix: Increase request timeout #102

wants to merge 1 commit into from

Conversation

denis-tingaikin
Copy link
Member

@denis-tingaikin denis-tingaikin commented Jan 27, 2021 •
edited
Loading

Signed-off-by: Denis Tingaikin [email protected]

Issue

Our nses have expiration 1 min by default
Our nscs have request timeout 15 sec by default

It could produce a wrong situation in the suite testing:

  1. the First test creates NSE
  2. the First test is done NSE is deleted (but the entry from the registry is not deleted! It will be deleted after NSE expiration)
  3. the Second test starts with NSE, NSC
  4. NSC in the Second test trying to connect to NSE from the First test!

If request timeout less than expiration from the first NSE then the test will fail due to NSC connect to died NSE.

Motivation

This PR provides tmp fix.

The real fix of the issue described here: networkservicemesh/sdk#666

@denis-tingaikin
increase request timeout
cd5dedd 
Signed-off-by: Denis Tingaikin <[email protected]>
@edwarnicke
Copy link
Member

This doesn't feel like a timer issue to me:

  1. the First test creates NSE
  2. the First test is done NSE is deleted (but the registry is not deleted if the second test run fast)

Whys is this? The NSE from the second test should end up with its own registration.

  1. the Second test starts with NSE, NSC
  2. NSC in the Second test trying to connect to NSE from the First test!

Sure... which should then cause an error, and cause falling through to the registration for the nexst registered NSE for that Network Service, which should be the second NSE.

Basically... this failure feels like something is very broken that has nothing to do with timers.

I don't see why we need additional mechanisms in networkservicemesh/sdk#666 to address this... it seems like a simple bug in existing mechanisms.

@denis-tingaikin
Copy link
Member Author

Whys is this? The NSE from the second test should end up with its own registration.

Yes, but the entry from the registry about the previous NSE could be deleted slowly for the current test.

@edwarnicke
Copy link
Member

Whys is this? The NSE from the second test should end up with its own registration.

Yes, but the entry from the registry about the previous NSE could be deleted slowly for the current test.

But that shouldn't matter... if the NSMgr tries to select the NSE from Test 1 (NSE1) it will fail, which will result in an error, and it will try for the NSE for Test 2 (which will succeed).

@denis-tingaikin denis-tingaikin mentioned this pull request Jan 28, 2021
Merged
nsmbot pushed a commit that referenced this pull request Nov 9, 2022
Update go.mod and go.sum to latest version from networkservicemesh/cm…
1885dcb 
…d-map-ip-k8s@main

PR link: networkservicemesh/cmd-map-ip-k8s#102

Commit: af2bce7
Author: Denis Tingaikin
Date: 2022-11-09 20:44:18 +0300
Message:
  - Merge pull request #102 from networkservicemesh/update/networkservicemesh/cmd-template
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit that referenced this pull request Jun 27, 2023
Update go.mod and go.sum to latest version from networkservicemesh/cm…
a7e4fd1 
…d-nsc-simple-docker@main

PR link: networkservicemesh/cmd-nsc-simple-docker#102

Commit: d4ce998
Author: Denis Tingaikin
Date: 2023-06-27 12:03:14 +0300
Message:
  - Merge pull request #102 from glazychev-art/govpp
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit that referenced this pull request Aug 10, 2023
Update go.mod and go.sum to latest version from networkservicemesh/cm…
85c45b0 
…d-nse-l7-proxy@main

PR link: networkservicemesh/cmd-nse-l7-proxy#102

Commit: bade16b
Author: Denis Tingaikin
Date: 2023-08-10 04:02:25 +0300
Message:
  - Merge pull request #102 from NikitaSkrynnik/release-act
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit that referenced this pull request Sep 27, 2024
Update go.mod and go.sum to latest version from networkservicemesh/cm…
bffebb4 
…d-lb-vl3-vpp@main

PR link: networkservicemesh/cmd-lb-vl3-vpp#102

Commit: ed113db
Author: Network Service Mesh Bot
Date: 2024-09-27 05:51:23 -0500
Message:
  - Update go.mod and go.sum to latest version from networkservicemesh/sdk-vpp@main (#102)
PR link: networkservicemesh/sdk-vpp#853
Commit: 0702bd9
Author: Network Service Mesh Bot
Date: 2024-09-27 05:48:04 -0500
Message:
    - Update go.mod and go.sum to latest version from networkservicemesh/sdk-kernel@main (#853)
PR link: networkservicemesh/sdk-kernel#681
Commit: 19add25
Author: Network Service Mesh Bot
Date: 2024-09-27 05:44:16 -0500
Message:
        - Update go.mod and go.sum to latest version from networkservicemesh/sdk@main (#681)
PR link: networkservicemesh/sdk#1670
Commit: b66e1bf
Author: Nikita Skrynnik
Date: 2024-09-27 17:37:34 +0700
Message:
                - Add more mutexes in dial chain element to fix race conditions (#1670)
* some minor change
* add more locks
---------
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit that referenced this pull request Dec 9, 2024
Update go.mod and go.sum to latest version from networkservicemesh/cm…
e45c4e7 
…d-csi-driver@main

PR link: networkservicemesh/cmd-csi-driver#102

Commit: 49415ab
Author: Network Service Mesh Bot
Date: 2024-12-09 05:45:00 -0600
Message:
  - Update go.mod and go.sum to latest version from networkservicemesh/sdk@main (#102)
PR link: networkservicemesh/sdk#1689
Commit: 1e611de
Author: Network Service Mesh Bot
Date: 2024-12-09 05:42:24 -0600
Message:
    - Update go.mod and go.sum to latest version from networkservicemesh/api@main (#1689)
PR link: networkservicemesh/api#180
Commit: bbb4cd5
Author: Denis Tingaikin
Date: 2024-12-09 03:03:53 -0500
Message:
        - update go to v1.23.3 (#180)
Signed-off-by: NSMBot <[email protected]>
nsmbot added a commit that referenced this pull request Dec 9, 2024
@nsmbot
Update go.mod and go.sum to latest version from networkservicemesh/cm…
61a3e8d 
…d-csi-driver@main (#12610)

PR link: networkservicemesh/cmd-csi-driver#102

Commit: 49415ab
Author: Network Service Mesh Bot
Date: 2024-12-09 05:45:00 -0600
Message:
  - Update go.mod and go.sum to latest version from networkservicemesh/sdk@main (#102)
PR link: networkservicemesh/sdk#1689
Commit: 1e611de
Author: Network Service Mesh Bot
Date: 2024-12-09 05:42:24 -0600
Message:
    - Update go.mod and go.sum to latest version from networkservicemesh/api@main (#1689)
PR link: networkservicemesh/api#180
Commit: bbb4cd5
Author: Denis Tingaikin
Date: 2024-12-09 03:03:53 -0500
Message:
        - update go to v1.23.3 (#180)

Signed-off-by: NSMBot <[email protected]>
Co-authored-by: NSMBot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
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
@denis-tingaikin @edwarnicke