Master discovery

[AnaNek]

tarantool/connection_pool allows users to use multiple connection and process request on replica/master instance according to specified mode.

Main features:

• Return available connection from pool according to round-robin strategy.
• Automatic master discovery by mode parameter.

Additional options (configurable via ConnectWithOpts):

• CheckTimeout - time interval to check for connection timeout and try to switch connection

Mode parameter:

• ANY (use any instance) - the request can be executed on any instance (master or replica).
• RW (writeable instance (master)) - the request can only be executed on master.
• RO (read only instance (replica)) - the request can only be executed on replica.
• PREFER_RO (prefer read only instance (replica)) - if there is one, otherwise fallback to a writeable one (master).
• PREFER_RW (prefer write only instance (master)) - if there is one, otherwise fallback to a read only one (replica).

Request	Default mode
call	no default
eval	no default
ping	no default
insert	RW
delete	RW
replace	RW
update	RW
upsert	RW
select	ANY
get	ANY

Example:

package main

import (
        "fmt"

	"github.com/tarantool/go-tarantool"
	"github.com/tarantool/go-tarantool/connection_pool"
)

func main() {
	resp, err := connPool.Eval("return box.info().ro", []interface{}{}, connection_pool.PreferRO)
	if err != nil {
		fmt.Printf("Failed to Eval", err.Error())
	}
	if resp == nil {
		fmt.Printf("Response is nil after Eval")
	}
	if len(resp.Data) < 1 {
		fmt.Printf("Response.Data is empty after Eval")
	}
	val := resp.Data[0].(bool)
	if val != true {
		fmt.Printf("Mode `PreferRO`: expected `true`, but got %v", val)
	}
}

To Do: health checks, balancing using weights/distances, anonymous replica

Closes #113

[DifferentialOrange]

It seems like a nice master discovery implementation.

I reviewed the main test file first and internals second, so some questions with "what does this mean" may looks weird.

Most of my comments are just a comments or a questions to clarify something. Since we don't have a well-structured approach to write test, feel free to ignore comments about asserts and not to dig to deep on deciding where we should use Errorf or Fatalt (even though I think it worth to at least replace all Errorf + return with Fatalf's). A couple of comments suggest to change or rework some things. I think it may be discussed separately if it is worth to do them now, later of ignore them.

One more time, thank you for your patch!

[DifferentialOrange]

It's just a part of review, I will add another comments today

[Totktonada]

It seems, it'll be hard for me to deeply dive into this pull request in the nearest future. OTOH, I'm interested mostly in high-level design and shared my vision in the RFC. If the implementation is correct and corresponds to the RFC (possibly with a gap to implement in a future), it likely will be okay for me.

I asked Georgy to do a decent review. Feel free to proceed after his approve.

[DifferentialOrange]

We strive to use not more than 50 symbols for commit message headline and 72 symbols for commit body lines: https://www.tarantool.io/en/doc/latest/dev_guide/developer_guidelines/#how-to-write-a-commit-message

For example, we may drop "automatic" and "connection-pool" headline since it does not adds any new info (and it's not like we have strict commit message style). At least let's try to fit to GitHub restrictions (seeing ellipsis is a bit painful)

Or maybe it's better to save the headline and use connection-pool: implement connection pool with master discovery, but at least fit to GitHub restrictions

[DifferentialOrange]

Let's collect the checklist from this PR comments and our live discussions:

• deal with futures Master discovery #148 (comment)
• separate the example and ensure that doc seems ok Master discovery #148 (comment)
• add some tests to ensure Typed and Async methods are ok
• prettify commit message Master discovery #148 (comment)

To be honest, I'm still doubt that we should use varargs to implement defaults instead of not implementing default at all. Yeah, defaults are cool if they are supported by language (or at least provide lightweight solution like in Lua), but if they require a separate function and variadic args to implement they stop being cool.

Have I forgot anything?

[AnaNek]

Let's collect the checklist from this PR comments and our live discussions:

• deal with futures Master discovery #148 (comment)
• separate the example and ensure that doc seems ok Master discovery #148 (comment)
• add some tests to ensure Typed and Async methods are ok
• prettify commit message Master discovery #148 (comment)

To be honest, I'm still doubt that we should use varargs to implement defaults instead of not implementing default at all. Yeah, defaults are cool if they are supported by language (or at least provide lightweight solution like in Lua), but if they require a separate function and variadic args to implement they stop being cool.

Have I forgot anything?

Done. I didn't test every Typed and Async methods, I suppose testing Typed and Async for one request type would be enough. You can find tests for SelectTyped and SelectAsync in connection_pool/example_test.go.

[DifferentialOrange]

And one last thing: there is a separate test target for each folder now

go-tarantool/Makefile

Line 24 in dd4f88f

test-queue:

Please, add one

[DifferentialOrange]

To be honest, I'm still doubt that we should use varargs to implement defaults instead of not implementing default at all. Yeah, defaults are cool if they are supported by language (or at least provide lightweight solution like in Lua), but if they require a separate function and variadic args to implement they stop being cool.

Regarding this: it seems that there are no noticeable performance drops. At least I couldn't catch them on shallow perf test. So looks like it is okay to leave it like this.

[DifferentialOrange]

Also it is possible to add comment to a subpackage

See

go-tarantool/uuid/uuid.go

Line 1 in dd4f88f

// Package with support of Tarantool's UUID data type.

for a solution

[Totktonada]

To be honest, I'm still doubt that we should use varargs to implement defaults instead of not implementing default at all. Yeah, defaults are cool if they are supported by language (or at least provide lightweight solution like in Lua), but if they require a separate function and variadic args to implement they stop being cool.

Maybe you're right. And it likely will look bad in the autogenerated docs. I see the following options:

• Options structure as the last agrument. Cons: I doubt that we'll need more options, we'll have many arguments.
• Don't add defaults.

[AnaNek]

And one last thing: there is a separate test target for each folder now

go-tarantool/Makefile

Line 24 in dd4f88f

test-queue:

Please, add one

Done.

[AnaNek]

Also it is possible to add comment to a subpackage

See

go-tarantool/uuid/uuid.go

Line 1 in dd4f88f

// Package with support of Tarantool's UUID data type.

for a solution

Done.

[DifferentialOrange]

Let's fix some minor doc issues and merge it at last. Great work!

[DifferentialOrange]

Let's merge it