tarantool/go-tarantool

Support IPROTO_PUSH

Closed this issue · 5 comments

box.session.push() in 1.10 impements a new iproto command, IPROTO_PUSH. Please add support for it.

yes, it will huge refactor. I will cobine it with new API.

unera commented

IPROTO_PUSH is broken by desing.

  • The mechanism doesn't have back-pressure.
  • The mechanism has ugly semantic (session against request).
  • The mechanism has wrong API name (session.push against multireturn. This is misleading to users.)

I think that we should

  • drop API session.push
  • rework the API to multireturn (add back-pressure)
  • and then add the requests to Go connectors.

I agree that box.session.push() design has several flaws (or, okay, maybe it had some idea behind that was newer implemented).

Details are here: tarantool/tarantool#6107.

In my humble opinion IPROTO_PUSH support would be convenient for notifications about a long request status despite highlighted problems. Another point is that box.session.push() is available since tarantool 1.10.2, but anything new will be only in new tarantool versions.

Anyway, Dmitry asked for a pause to think and decide what is better to do here and, well, 'think than do' is better than vice versa :)

NB: https://github.com/FZambia/tarantool supports box.session.push(), so we can look at the API.

We can add a stateful iterator with an interface

type ResponseIterator interface {
	Next() (bool)
	Value() (*Response)
	Err() (error)
}

and a function to get an interface implementation from the Future type:

func (fut *Future) GetIterator() ResponseIterator {
	// creates and returns an object of private type that implements ResponseIterator
}

Then a client-side code will look like this:

var fut *Future
var it ResponseIterator
var err error

fut = conn.CallAsync("server_function", []interface{}{})
for it = fut.GetIterator(); it.Next(); {
	resp := it.Value()
	if (resp.Code == PushCode) { // PushCode == IPROTO_PUSH == 0x80
		// push message
	} else {
		// regular response
	}
}
if err = it.Err(); err != nil {
	// an error happens
}

for it = fut.GetIterator().WithTimeout(1 * time.Second); it.Next(); {
	resp := it.Value()
	if (resp.Code == PushCode) { // PushCode == IPROTO_PUSH == 0x80
		// push message
	} else {
		// regular response
	}
}
if err = it.Err(); err != nil {
	// an error happens
}

The code handle an asynchronous case from a Lua implementation in the same way, see examples on Lua.

A synchronous case will require much more changes. If we try to extend the current implementation with additional functions for passing callbacks, it will not look very nice with a lot of duplication code. It seems to me that now it would be better to support only the asynchronous case.