-
Notifications
You must be signed in to change notification settings - Fork 60
pushAPI: remove box.session.push usage #494
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,8 @@ TODO | |||||
| * `box.New` returns an error instead of panic | ||||||
| * Added `box.MustNew` wrapper for `box.New` without an error | ||||||
| * Removed deprecated `pool` methods, related interfaces and tests are updated. | ||||||
| * Removed `box.session.push()` usage: Future.AppendPush() and Future.GetIterator() | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| methods, ResponseIterator and TimeoutResponseIterator types, `Future.pushes[]`. | ||||||
|
|
||||||
| ## Migration from v1.x.x to v2.x.x | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -882,15 +882,7 @@ func (conn *Connection) reader(r io.Reader, c Conn) { | |
| } | ||
| continue | ||
| } else if code == iproto.IPROTO_CHUNK { | ||
| if fut = conn.peekFuture(header.RequestId); fut != nil { | ||
| if err := fut.AppendPush(header, &buf); err != nil { | ||
| err = ClientError{ | ||
| ErrProtocolError, | ||
| fmt.Sprintf("failed to append push response: %s", err), | ||
| } | ||
| conn.opts.Logger.Report(LogAppendPushFailed, conn, err) | ||
| } | ||
| } | ||
| // not implemented | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: we need to add a log message here, something like:
and a test for it.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After removing Lua push_func, the IPROTO_CHUNK occurrence is no longer possible. Maybe we need to completely remove IPROTO_CHUNK mention? (furthermore, I've already written in changelog.md that I deleted it) |
||
| } else { | ||
| if fut = conn.fetchFuture(header.RequestId); fut != nil { | ||
| if err := fut.SetResponse(header, &buf); err != nil { | ||
|
|
@@ -1131,26 +1123,6 @@ func (conn *Connection) markDone(fut *Future) { | |
| conn.decrementRequestCnt() | ||
| } | ||
|
|
||
| func (conn *Connection) peekFuture(reqid uint32) (fut *Future) { | ||
| shard := &conn.shard[reqid&(conn.opts.Concurrency-1)] | ||
| pos := (reqid / conn.opts.Concurrency) & (requestsMap - 1) | ||
| shard.rmut.Lock() | ||
| defer shard.rmut.Unlock() | ||
|
|
||
| if conn.opts.Timeout > 0 { | ||
| if fut = conn.getFutureImp(reqid, true); fut != nil { | ||
| pair := &shard.requests[pos] | ||
| *pair.last = fut | ||
| pair.last = &fut.next | ||
| fut.timeout = time.Since(epoch) + conn.opts.Timeout | ||
| } | ||
| } else { | ||
| fut = conn.getFutureImp(reqid, false) | ||
| } | ||
|
|
||
| return fut | ||
| } | ||
|
|
||
| func (conn *Connection) fetchFuture(reqid uint32) (fut *Future) { | ||
| shard := &conn.shard[reqid&(conn.opts.Concurrency-1)] | ||
| shard.rmut.Lock() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.