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.

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

Fixes thread safety bug on in-memory storage regarding accept count #2779

Merged
merged 2 commits into from Aug 24, 2019

Conversation

codefromthecrypt
Copy link
Member

Before, we were using an unsafe means to increment a counter for the
in-memory storage. This contributed to a test flake. This fixes the
state bug and also tightens up the flakey test.

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Code and test code LGTM

@codefromthecrypt
Copy link
Member Author

so still flaking on one endpoint which isn't returning a span for the storage layer..

  [
    {
      "traceId": "7dddc296c13a5448",
      "id": "7dddc296c13a5448",
      "kind": "SERVER",
      "name": "post /api/v1/spans",
      "timestamp": 1566632878396438,
      "duration": 14240,
      "localEndpoint": {
        "serviceName": "zipkin-server",
        "ipv4": "172.17.0.1"
      },
      "remoteEndpoint": {
        "ipv4": "127.0.0.1",
        "port": 57474
      },
      "annotations": [
        {
          "timestamp": 1566632878396451,
          "value": "wr"
        },
        {
          "timestamp": 1566632878410664,
          "value": "ws"
        }
      ],
      "tags": {
        "http.method": "POST",
        "http.path": "/api/v1/spans"
      }
    }
  ]

Adrian Cole added 2 commits August 24, 2019 19:43
Before, we were using an unsafe means to increment a counter for the
in-memory storage. This contributed to a test flake. This fixes the
state bug and also tightens up the flakey test.

The flakey test still exists, so is disabled for now.
@codefromthecrypt codefromthecrypt merged commit afd63b5 into master Aug 24, 2019
@codefromthecrypt codefromthecrypt deleted the read-bug branch August 24, 2019 13:27
abesto pushed a commit to abesto/zipkin that referenced this pull request Sep 10, 2019
…penzipkin#2779)

Before, we were using an unsafe means to increment a counter for the
in-memory storage. This contributed to a test flake. This fixes the
state bug and also tightens up the flakey test.

The flakey test still exists, so is disabled for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants