Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1093#discussion_r80467634
--- Diff: lib/php/lib/Thrift/Protocol/TBinaryProtocol.php ---
@@ -246,6 +257,10 @@ public function readMessageBegin(&$name, &$type,
&$seqid)
}
}
+ if ($seqid != $this->seqid_) {
+ throw new TApplicationException("TBinaryProtocol::ReadMessageBegin
received SequenceID: $seqid not matches requested ID: $this->seqid_ " .
TApplicationException::BAD_SEQUENCE_ID);
+ }
--- End diff --
The sequence ID was originally put in place for future expansion of the
servers to allow for multiple outstanding requests from a client. Most servers
do not implement this. For example on the C++ threaded servers there is one
request per client at any given time. The C# server is the same way. The
servers could reply with a seqId that matches the request the client sent,
allowing a client to submit multiple requests, if the servers were written
differently. Most client implementations ignore seqId because they only issue
one request at a time and not all servers fill in the field properly. The
seqId should be handled on the server as a connection-specific unique request
id.
My recommendation would be to follow what other clients do and atomically
increment a number starting at 1. This will be safer than random. It probably
isn't safe to check that the server's response seqId equals the last request
sent. Some servers may not fill in the field. This also limits your client to
one concurrent request and hampers future expansion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
