Create mechanism for plugin to store and share state with other plugins in the same cluster. #3

Jacobingalls · 2017-01-21T07:53:32Z

Adds the ability to store volumes via drivers. There are several included drivers

Drivers:

in-memory: For testing purposes, keeps the entire state in memory. Completely ephemeral.
sqlite: Stores the state of the plugin in a local database file, this could be shared between plugins via nfs or gluster fs.
mysql: Store the state in a remote mysql server, this guarantees consistency but requires an additional service/container/server to maintain.

The default is sqlite and can be changed with -db

Example:

./run.sh -db=mysql -dbpass=mypassword -dbschema=rdma_volumes

This starts the plugin with mysql, counting to localhost with root user, mypassword as the password, and the schema as rdma_volumes.

…n the hosts disk

…ved and never mounted

…-volume-rdma into HEAD

JohnStarich · 2017-01-23T01:14:15Z

.gitignore

@@ -91,3 +91,4 @@ $RECYCLE.BIN/
 # End of https://www.gitignore.io/api/macos,windows,linux

 .idea
+*.db


For shame on missing the newline 😉

I honestly don't know what you are talking about.

Click on the file name to see it. Looks like the review comment hid the red mark

JohnStarich · 2017-01-23T23:01:29Z

db/inmemory.go

+
+// List all of the volumes in the database, returning an error if one occured.
+func (i InMemoryVolumeDatabase) List() ([]*volume.Volume, error) {
+	l := make([]*volume.Volume, 0, len(i.volumes))


Short variable names hinder the reader's ability to understand this code. Please lengthen them for all our sakes.

JohnStarich · 2017-01-23T23:03:05Z

db/mysql.go

+		glog.Fatal("A database schema must be specified with -dbschema")
+	}
+	connection += "/" + schema
+	printedConnection += "/" + schema


Please use path.Join instead

JohnStarich · 2017-01-23T23:03:54Z

drivers/driver.go

@@ -36,8 +25,8 @@ type RDMAStorageController interface {
 }

 // NewRDMAVolumeDriver constructs a new RDMAVolumeDriver.
-func NewRDMAVolumeDriver(sc RDMAStorageController) RDMAVolumeDriver {
-	return RDMAVolumeDriver{sc}
+func NewRDMAVolumeDriver(sc RDMAStorageController, vd db.VolumeDatabase) RDMAVolumeDriver {


Short variable names hinder the reader's ability to understand this code. Please lengthen them for all our sakes.

I think this is clear.

JohnStarich · 2017-01-23T23:08:47Z

db/sqldb.go

+	}
+
+	// Begin transaction to the database
+	tx, err := sqlDB.Begin()


Short variable names hinder the reader's ability to understand this code. Please lengthen them for all our sakes.

Fixed in later commit.

JohnStarich · 2017-01-23T23:15:23Z

main.go


 	// Create and begin serving volume driver on tcp/ip port, httpPort.
+	vd, dberr := GetDatabaseConnection()


Short variable names hinder the reader's ability to understand this code. Please lengthen them for all our sakes.

Fixed in later commit.

JohnStarich · 2017-01-23T23:15:44Z

main.go

+		glog.Fatal(dberr)
+	}
+
+	// Configure Storage Controller
 	sc := drivers.NewGlusterStorageController()


Short variable names hinder the reader's ability to understand this code. Please lengthen them for all our sakes.

Fixed in later commit.

JohnStarich · 2017-01-23T23:18:58Z

main.go

+	defer driver.Disconnect()
+
+	// Handle SIGINT gracefully
+	c := make(chan os.Signal, 2)


Short variable names hinder the reader's ability to understand this code. Please lengthen them for all our sakes.

JohnStarich · 2017-01-23T23:19:11Z

main.go

+		os.Exit(1)
+	}()
+
+	glog.Info("Running! http://localhost:" + port)
 	h := volume.NewHandler(driver)


Short variable names hinder the reader's ability to understand this code. Please lengthen them for all our sakes.

Fixed in later commit.

JohnStarich · 2017-01-23T23:26:23Z

main.go

+func GetDatabaseConnection() (db.VolumeDatabase, error) {
+	switch volumeDatabaseDriver {
+	case "in-memory":
+		validateDatabaseFlags(true, true, false, false, false, false)


After some brief searching I found this: https://groups.google.com/forum/#!topic/golang-nuts/oWeeqCJfRzk

They basically made a config struct and passed the struct after initialization around. Performance-wise this would be identical, but reading-wise it would be far more obvious what's going on. Let's do that instead

Created #13

…er-volume-rdma into feature/db

…-volume-rdma into feature/db

Jacobingalls added 11 commits January 18, 2017 22:33

Implment an in-memory volume database

4a91710

Allow run.sh to accept arguments and pass them to the plugin

aeccddd

Add WIP sqlite implementation

72c0b39

Update documentation to be more clear

f75461d

Exit gracefully on SIGINT

755ecfd

Add support for mysql and clean up overall implementation

0a577c6

Update main.go to use mellanox-senior-design (with db).

68aac6c

Implement on-disk driver that allows creation of volumes as folders o…

c0bd5ae

…n the hosts disk

Use more descriptive variable names

1920109

Fix issue preventing scpath from being used on sc=on-disk

8704fa7

Fix bug where on-disk driver would return an error if volume was remo…

12d63a0

…ved and never mounted

Jacobingalls force-pushed the feature/db branch from 68aac6c to fb9c7eb Compare January 23, 2017 22:05

Update main.go to use mellanox-senior-design (with db).

8c5f903

Jacobingalls force-pushed the feature/db branch from fb9c7eb to 8c5f903 Compare January 23, 2017 22:09

Merge branch 'feature/db' of github.com:mellanox-senior-design/docker…

310993c

…-volume-rdma into HEAD

JohnStarich force-pushed the feature/db branch from 1006f71 to 9b56bbb Compare January 23, 2017 22:32

Fix build status

ea71ff1

JohnStarich force-pushed the feature/db branch from 9b56bbb to ea71ff1 Compare January 23, 2017 22:39

JohnStarich requested changes Jan 23, 2017

View reviewed changes

Jacobingalls added 4 commits February 6, 2017 12:53

Add newline to the end of the .gitignore

dca7b49

Changes from code review

674080b

Use better join in mysql config

03508fb

Add ; to sql that was missing it

f09ec7c

JohnStarich approved these changes Feb 6, 2017

View reviewed changes

JohnStarich and others added 3 commits February 6, 2017 14:11

Merge branch 'develop' into feature/db

5d10367

Merge branch 'feature/disk' of github.com:mellanox-senior-design/dock…

832d5b3

…er-volume-rdma into feature/db

Merge branch 'feature/db' of github.com:mellanox-senior-design/docker…

c18c790

…-volume-rdma into feature/db

JohnStarich approved these changes Feb 6, 2017

View reviewed changes

Jacobingalls merged commit 45a16d3 into develop Feb 6, 2017

JohnStarich deleted the feature/db branch February 6, 2017 20:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create mechanism for plugin to store and share state with other plugins in the same cluster. #3

Create mechanism for plugin to store and share state with other plugins in the same cluster. #3

Jacobingalls commented Jan 21, 2017

JohnStarich Jan 23, 2017

Jacobingalls Jan 24, 2017

JohnStarich Jan 24, 2017

Jacobingalls Feb 6, 2017 •

edited

Loading

JohnStarich Jan 23, 2017

Jacobingalls Feb 6, 2017

JohnStarich Jan 23, 2017

Jacobingalls Feb 6, 2017

JohnStarich Jan 23, 2017

Jacobingalls Feb 6, 2017 •

edited

Loading

JohnStarich Jan 23, 2017

Jacobingalls Feb 6, 2017

JohnStarich Jan 23, 2017

Jacobingalls Feb 6, 2017

JohnStarich Jan 23, 2017

Jacobingalls Feb 6, 2017

JohnStarich Jan 23, 2017

JohnStarich Jan 23, 2017

Jacobingalls Feb 6, 2017

JohnStarich Jan 23, 2017

Jacobingalls Feb 6, 2017


		// Create and begin serving volume driver on tcp/ip port, httpPort.
		vd, dberr := GetDatabaseConnection()

Create mechanism for plugin to store and share state with other plugins in the same cluster. #3

Create mechanism for plugin to store and share state with other plugins in the same cluster. #3

Conversation

Jacobingalls commented Jan 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jacobingalls Feb 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jacobingalls Feb 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jacobingalls Feb 6, 2017 •

edited

Loading

Jacobingalls Feb 6, 2017 •

edited

Loading