Skip to content
This repository was archived by the owner on Feb 18, 2025. It is now read-only.

Replaced stopwatch with a shared-stopwatch suitable for sharing between go routines#298

Open
shlomi-noach wants to merge 4 commits intomasterfrom
shared-stopwatch
Open

Replaced stopwatch with a shared-stopwatch suitable for sharing between go routines#298
shlomi-noach wants to merge 4 commits intomasterfrom
shared-stopwatch

Conversation

@shlomi-noach
Copy link
Collaborator

@shlomi-noach shlomi-noach commented Sep 17, 2017

resubmission of #273 by @maurosr , which I merged into this branch for testing.

Mauro Schilman and others added 3 commits August 22, 2017 12:00
Replaced stopwatch with a shared-stopwatch suitable for sharing betwe…
6cb308b 
…en go routines.

Fixed discovery timing metrics.
Fixed json encoding of discovery metrics.
Merge branch 'master' into master
72d1a19
Merge pull request #273 from maurosr/master
579d9e0 
Replaced stopwatch with a shared-stopwatch suitable for sharing between go routines
@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=concertmaster September 17, 2017 07:03 Inactive
@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=conductor September 17, 2017 07:44 Inactive
@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=concertmaster September 17, 2017 08:02 Inactive
@shlomi-noach shlomi-noach temporarily deployed to production/mysql_cluster=conductor September 17, 2017 08:02 Inactive
@shlomi-noach
Copy link
Collaborator Author

shlomi-noach commented Sep 17, 2017

I'm getting reproducible nil pointer errors:

# orchestrator -debug -c relocate -i server.1.com:3306 -d server.2.com --ignore-raft-setup
2017-09-17 01:03:18 INFO starting orchestrator, version: 579d9e08fdfad13bb68f7ec6438ab08ca8aec9e4
2017-09-17 01:03:18 INFO Read config: /etc/orchestrator.conf.json
2017-09-17 01:03:18 DEBUG Parsed orchestrator credentials from /etc/mysql/orchestrator_srv.cnf
2017-09-17 01:03:18 DEBUG Parsed topology credentials from /etc/mysql/orchestrator.cnf
2017-09-17 01:03:18 DEBUG Hostname unresolved yet: server.1.com
2017-09-17 01:03:18 DEBUG Cache hostname resolve server.1.com as server.1.com
2017-09-17 01:03:18 DEBUG Hostname unresolved yet: server.2.com
2017-09-17 01:03:18 DEBUG Cache hostname resolve server.2.com as server.2.com
2017-09-17 01:03:18 DEBUG Connected to orchestrator backend: orchestrator_srv:?@tcp(127.0.0.1:3306)/orchestrator?timeout=1s
2017-09-17 01:03:18 DEBUG Orchestrator pool SetMaxOpenConns: 128
2017-09-17 01:03:18 DEBUG Initializing orchestrator
2017-09-17 01:03:18 INFO Connecting to backend: maxConnections: 128, maxIdleConns: 32
2017-09-17 01:03:18 DEBUG WriteResolvedHostname: resolved server.2.com to server.2.com
2017-09-17 01:03:18 DEBUG WriteResolvedHostname: resolved server.1.com to server.1.com
2017-09-17 01:03:18 ERROR ReadTopologyInstance(server.1.com:3306) Unexpected, aborting: runtime error: invalid memory address or nil pointer dereference
2017-09-17 01:03:18 ERROR ReadTopologyInstance(server.2.com:3306) Unexpected, aborting: runtime error: invalid memory address or nil pointer dereference
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4e610b]

goroutine 1 [running]:
panic(0xa49b80, 0xc42001e140)
        /usr/local/go/src/runtime/panic.go:500 +0x1a1
github.com/github/orchestrator/go/inst.MatchBelow(0xc4202ec840, 0xc4202ecdc0, 0xc420000001, 0x0, 0x0, 0x0, 0x0)
        github.com/github/orchestrator/go/inst/instance_topology.go:1391 +0x22b
github.com/github/orchestrator/go/inst.relocateBelowInternal(0xc4202ec840, 0xc4202ecdc0, 0x1, 0x0, 0x0)
        github.com/github/orchestrator/go/inst/instance_topology.go:2618 +0xcc4
github.com/github/orchestrator/go/inst.RelocateBelow(0xc4202ec2c0, 0xc4202ce2c0, 0xc4202ec2c0, 0x0, 0x0)
        github.com/github/orchestrator/go/inst/instance_topology.go:2656 +0x21f
github.com/github/orchestrator/go/app.Cli(0x7ffe772ab9a4, 0x8, 0x0, 0x7ffe772ab9b0, 0x2a, 0xc4201a1ec0, 0x28, 0x0, 0x0, 0x0, ...)
        github.com/github/orchestrator/go/app/cli.go:221 +0x1e220
github.com/github/orchestrator/go/app.CliWrapper(0x7ffe772ab9a4, 0x8, 0x0, 0x7ffe772ab9b0, 0x2a, 0x7ffe772ab9de, 0x23, 0x0, 0x0, 0x0, ...)
        github.com/github/orchestrator/go/app/cli.go:149 +0x23e
main.main()
        github.com/github/orchestrator/go/cmd/orchestrator/main.go:162 +0xd0c

These do not happen with the master branch. cc @maurosr

@shlomi-noach shlomi-noach mentioned this pull request Sep 17, 2017
Merged
@shlomi-noach
Copy link
Collaborator Author

shlomi-noach commented Sep 17, 2017

I did see multiple occurances of

src/github.com/github/orchestrator/vendor/github.com/codegangsta/inject/inject.go:102 (0x7d8e25)
/usr/local/go/src/runtime/asm_amd64.s:480 (0x46468c)
        call64: CALLFN(·call64, 64)
/usr/local/go/src/reflect/value.go:434 (0x52f128)
        Value.call: call(frametype, fn, args, uint32(frametype.size), uint32(retOffset))
/usr/local/go/src/reflect/value.go:302 (0x52ea44)
        Value.Call: return v.call("Call", in)

Is it possible that the metrics fail the ReadInstance method and make it return nil?

@shlomi-noach shlomi-noach requested a review from sjmudd September 17, 2017 08:16
@shlomi-noach shlomi-noach mentioned this pull request Sep 17, 2017
Closed
@maurosr
Copy link

maurosr commented Sep 18, 2017

I can see where the bug is. I am very sorry, will fix this promptly.

maurosr pushed a commit to maurosr/orchestrator that referenced this pull request Sep 18, 2017
Added DummyNamedStopwatch to avoid bug reported in openark#298.
05aa224
@maurosr
Copy link

maurosr commented Sep 18, 2017

Can you please cherry-pick this commit maurosr@05aa224 and check if the error is fixed? Thanks!

(I will use another branch for this kind of changes in the future)

@shlomi-noach
Copy link
Collaborator Author

shlomi-noach commented Oct 17, 2017

Sorry, I was out for quite a while ; I will pick this up in a few days.

@sjmudd
Copy link
Collaborator

sjmudd commented Nov 15, 2017

Hi Shlomi. Is there any chance you can look at this?

@sjmudd sjmudd mentioned this pull request Nov 15, 2017
Closed
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sjmudd sjmudd Awaiting requested review from sjmudd

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shlomi-noach @maurosr @sjmudd