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

boltdb/bolt==>bbolt, revendor Microsoft/*, Windows compilation. #2268

Merged
merged 6 commits into from
Sep 13, 2018

Conversation

lowenna
Copy link
Member

@lowenna lowenna commented Sep 12, 2018

Signed-off-by: John Howard jhoward@microsoft.com

Almost fixes #1950.

Most of this PR is vendoring.

Once all this is merged, we need a follow-on PR to revendor libnetwork into moby/moby to finally fix #1950 (which should really be open in the moby/moby repo, but not important.)

@carlfischer1 FYI

@mavenugo PTAL

John Howard added 2 commits September 12, 2018 14:05
Signed-off-by: John Howard <jhoward@microsoft.com>
Signed-off-by: John Howard <jhoward@microsoft.com>
@lowenna
Copy link
Member Author

lowenna commented Sep 12, 2018

Could someone help me understand what this failure might mean? Or help debug it? I'm not that familiar with libnetwork. Tentatively possible is a regression due to boltdb, purely based on supposition in that it has db in the name of the test.

--- FAIL: TestNetworkDBCRUDMediumCluster (16.92s)
	networkdb_test.go:112: Network existence verification failed
	networkdb_test.go:136: Entry existence verification test failed for node3(5e292e4e65e9)
	networkdb_test.go:136: Entry existence verification test failed for node3(5e292e4e65e9)
	networkdb_test.go:455: assertion failed: expression is false: strings.Contains(err.Error(), "deleted and pending garbage collection")

@lowenna
Copy link
Member Author

lowenna commented Sep 12, 2018

I suspect this is a flaky test. It doesn't look like NetworkDB uses bolt (bbolt). On a second run, it failed in a different way:

=== RUN   TestNetworkDBCRUDMediumCluster
--- FAIL: TestNetworkDBCRUDMediumCluster (2.06s)
	networkdb_test.go:57: Number of nodes for node3 into the cluster does not match 5 != 4

I'll give it a third CI attempt...

@lowenna
Copy link
Member Author

lowenna commented Sep 12, 2018

Ha, third time it passed. Pretty sure that's just a flaky test 😞

@thaJeztah FYI.

@mavenugo
Copy link
Contributor

Yes. I dont know why and how networkdb fails for boltdb changes.
@fcrisciani might know more details on this.

Copy link
Contributor

@mavenugo mavenugo left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

Could you squash these three commits? Looks like they should go together;

  • Bump libkv to 458977154600b9f23984d9f4b82e79570b5ae12b
  • Remove github.com/boltdb/bolt from vendor.conf
  • Vendor go.etcd.io/bbolt @ v1.3.1-etcd.8

John Howard added 4 commits September 13, 2018 09:20
Signed-off-by: John Howard <jhoward@microsoft.com>

As well as bumping, libkv now requires go.etcd.io/bolt rather
than boltdb/bolt. Hence removed bolt from vendor.conf,
vendored go.etcd.io/bbot @ v1.3.1-etcd.8 and rerun vndr.
Signed-off-by: John Howard <jhoward@microsoft.com>
Signed-off-by: John Howard <jhoward@microsoft.com>
Signed-off-by: John Howard <jhoward@microsoft.com>
@lowenna
Copy link
Member Author

lowenna commented Sep 13, 2018

Could you squash these three commits?

@thaJeztah Done

@selansen
Copy link
Collaborator

LGTM

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

thanks!

@lowenna
Copy link
Member Author

lowenna commented Sep 13, 2018

Who can merge this? I'd really like to get the fix into moby/moby due to the number of people hitting the issue

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.

Saving endpoint to store fails in windows under concurrent load
5 participants