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

Move to Go 1.5 #3863

Merged
merged 5 commits into from
Aug 27, 2015
Merged

Move to Go 1.5 #3863

merged 5 commits into from
Aug 27, 2015

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Aug 27, 2015

This patch migrates InfluxDB to Go 1.5.

Very little had to change. The most significant change was to some unit test code, in which an interaction with Go's race detector was exposed. This was addressed using some explicit synchronization. The unit test was also split in two for clarity.

Only one small change to production code was required -- the explicit configuration of GOMAXPROCS.

@otoolep
Copy link
Contributor Author

otoolep commented Aug 27, 2015

Fixes #3731

@otoolep otoolep force-pushed the move_to_go1.5 branch 4 times, most recently from 24cd3c1 to 5529327 Compare August 27, 2015 01:07
@otoolep
Copy link
Contributor Author

otoolep commented Aug 27, 2015

@@ -517,8 +517,8 @@ func TestClient_Timeout(t *testing.T) {
_, err = c.Query(query)
if err == nil {
t.Fatalf("unexpected success. expected timeout error")
} else if !strings.Contains(err.Error(), "use of closed network connection") {
t.Fatalf("unexpected error. expected 'use of closed network connection' error, got %v", err)
} else if !strings.Contains(err.Error(), "request canceled") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@corylanou -- please confirm that this is OK with you. I think the error must have changed in Go 1.5.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's interesting. It appears to be a "better" error now. I don't remember reading anything in the release notes on it, but it's probably in their changelog. Seems fine. +1

@otoolep
Copy link
Contributor Author

otoolep commented Aug 27, 2015

@aviau -- scrap that. Can you please take a look at this build break, and tell me what you think is going on? It seems to think it's overwriting go vet but we don't explicitly install it.

dpkg: error processing archive /var/cache/apt/archives/golang-go.tools_0.0~hg20131126-3_i386.deb (--unpack):
 trying to overwrite '/usr/lib/go/pkg/tool/linux_386/vet', which is also in package golang-go 2:1.5
dpkg-deb: error: subprocess paste was killed by signal (Broken pipe)
Errors were encountered while processing:
 /var/cache/apt/archives/golang-go.tools_0.0~hg20131126-3_i386.deb
E: Sub-process /usr/bin/dpkg returned an error code (1)
lxc-start: lxc_start.c: main: 342 The container failed to start.
lxc-start: lxc_start.c: main: 346 Additional information can be obtained by setting the --logfile and --logpriority options.

@aviau
Copy link
Contributor

aviau commented Aug 27, 2015

@otoolep

I think this is #1486560.

I could fix it like this:

From 6040124ee810934750684a8f6e6d7f0b79fdc21a Mon Sep 17 00:00:00 2001
From: aviau <alexandre@alexandreviau.net>
Date: Thu, 27 Aug 2015 09:29:17 -0400
Subject: [PATCH] Fix 32bit Dockerfile

---
 Dockerfile_test_ubuntu32 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Dockerfile_test_ubuntu32 b/Dockerfile_test_ubuntu32
index 96c5a0b..672860a 100644
--- a/Dockerfile_test_ubuntu32
+++ b/Dockerfile_test_ubuntu32
@@ -1,6 +1,7 @@
 FROM 32bit/ubuntu:14.04

 RUN apt-get update && apt-get install -y python-software-properties software-properties-common git
+RUN apt-get remove -y golang-go
 RUN add-apt-repository ppa:evarlast/golang1.5
 RUN apt-get update && apt-get install -y golang-go

-- 
2.5.0

@aviau
Copy link
Contributor

aviau commented Aug 27, 2015

I opened a PR. #3867

Lets wait for the build.

From
golang/go@516f0d1

"Note that we use a channel send here and not a close.
The race detector doesn't know that we're waiting for a timeout
and thinks that the waitgroup inside httptest.Server is added to
concurrently with us closing it. If we timed out immediately, we could close
the testserver before we entered the handler. We're not timing out immediately
and there's no way we would be done before we entered the handler, but the
race detector doesn't know this, so synchronize explicitly.
confignotimeout := client.Config{URL: *u}
cnotimeout, err := client.NewClient(confignotimeout)
_, err = cnotimeout.Query(query)
func TestClient_NoTimeout(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TestClientTimeout has been split into two tests.

@otoolep
Copy link
Contributor Author

otoolep commented Aug 27, 2015

Green build, ready to make the move to Go 1.5.

@corylanou
Copy link
Contributor

+1

2 similar comments
@DanielMorsing
Copy link
Contributor

+1

@pauldix
Copy link
Member

pauldix commented Aug 27, 2015

+1

otoolep added a commit that referenced this pull request Aug 27, 2015
@otoolep otoolep merged commit afa3cd0 into master Aug 27, 2015
@otoolep otoolep deleted the move_to_go1.5 branch September 2, 2015 21:53
@otoolep otoolep mentioned this pull request Nov 11, 2015
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.

5 participants