Skip to content

test: add concurrency test - #8

Draft
taras-zak wants to merge 1 commit into
mainfrom
taras/bug-in-subscription
Draft

test: add concurrency test#8
taras-zak wants to merge 1 commit into
mainfrom
taras/bug-in-subscription

Conversation

@taras-zak

@taras-zak taras-zak commented Jan 10, 2025

Copy link
Copy Markdown

Pull Request Submission Checklist

Please confirm that you have done the following before requesting reviews:

  • I have confirmed that the PR type is appropriate for the change I am making according to
    the Honest Pull Request and Commit Message Naming Conventions.
  • I have typed an adequate description that explains why I am making this change.
  • I have installed and run standard pre-commit hooks that lints and validates my code.

Description

  • Redis subscriptions have at-most-once semantic and publishing event for topic that not have subscribers will lead to missing this event. TestPublishBeforeSubscribe cover this behaviour, we can see that "before subscribe" event (even "publish immediately" sometimes) not reached subscriber:
=== RUN   TestPublishBeforeSubscribe
2025/01/10 13:04:38 after subscribe
--- PASS: TestPublishBeforeSubscribe (5.01s)
PASS
=== RUN   TestPublishBeforeSubscribe
2025/01/10 13:07:20 publish immediately
2025/01/10 13:07:20 after subscribe
--- PASS: TestPublishBeforeSubscribe (5.01s)
PASS
  • So cache-lib have bug, when concurrent requests might stuck waiting notification for event. Here is results of the TestConcurrentRequestsForSameKey runs, we can see that it not stable:
=== RUN   TestConcurrentRequestsForSameKey
2025/01/10 12:58:24 miss from 1
2025/01/10 12:58:25 first request finished &{true} <nil>
2025/01/10 12:58:30 second request finished <nil> error reading from pub/sub
    cache_test.go:250: 
        	Error Trace:	/Users/taras/projects/cache-lib-go/cache_test.go:250
        	Error:      	Received unexpected error:
        	            	error reading from pub/sub
        	Test:       	TestConcurrentRequestsForSameKey
--- FAIL: TestConcurrentRequestsForSameKey (5.11s)

FAIL

Another run might pass

=== RUN   TestConcurrentRequestsForSameKey
2025/01/10 13:01:58 miss from 1
2025/01/10 13:01:58 first request finished &{true} <nil>
2025/01/10 13:01:58 second request finished &{true} <nil>
--- PASS: TestConcurrentRequestsForSameKey (0.11s)
PASS
  • fnx-api-gateway has ~100 errors per hour during peak time because of this bug - grafana

This change is Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant