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

Add UseCompressionOnStreaming parameter #83

Merged
merged 1 commit into from Aug 28, 2014

Conversation

Projects
None yet
2 participants
@a4lg
Contributor

a4lg commented Aug 28, 2014

UseCompression is very useful on non-streaming request. However, this will lead to the undesirable behavior on streaming requests because Twitter server 'buffers' messages and there are no ways to determine how many messages 'buffered'.
This commit adds UseCompressionOnStreaming option [default false] to make streaming behavior desirable and predictable.

――のように、ConnectionOptions 中の UseCompression プロパティが streaming リクエストにおいて予期しない動作を生み出す問題をオプションの追加により修正します。
どうやら Twitter サーバは gzip を受け取る user stream においてメッセージをバッファするらしく、リアルタイムでメッセージを取得できなくなります。その上、このバッファされたメッセージ数を知る方法が存在せず、接続切断時などに予測出来ない量のメッセージが消失します。これは情報収集系の bot 作成に悪影響をもたらします。そのため UseCompressionOnStreaming オプションを追加し、当面の問題に対処します。

――が、マージする前に確認できていない問題をこれを書いている最中に発見 (Stream 側でバッファされているせいで Twitter サーバから送信されている情報を十分に取れない可能性がある) しました。こちらの場合、上記の対処手段は workaround 以上の意味がない (むしろ無駄にコードを増やす有害な) ものになってしまいます。これをポストしてからパケットキャプチャでテストしてみます。それまではマージをしないようにお願い致します。

@a4lg

This comment has been minimized.

Show comment
Hide comment
@a4lg

a4lg Aug 28, 2014

Contributor

早速ですが、一旦 close します。

  • Clone メソッドの実装で UseCompressionOnStreaming のコピーをしていない凡ミスがあった
  • WireShark で観察した結果、サーバからは圧縮された情報が都度送られている様子だった

原因としては Response の Stream が gzip 結果をバッファするためであり、このバッファをどうにかして切らないと予想した通りの動作にならないと考えます。

Contributor

a4lg commented Aug 28, 2014

早速ですが、一旦 close します。

  • Clone メソッドの実装で UseCompressionOnStreaming のコピーをしていない凡ミスがあった
  • WireShark で観察した結果、サーバからは圧縮された情報が都度送られている様子だった

原因としては Response の Stream が gzip 結果をバッファするためであり、このバッファをどうにかして切らないと予想した通りの動作にならないと考えます。

@a4lg a4lg closed this Aug 28, 2014

Add UseCompressionOnStreaming parameter
UseCompression is very useful on non-streaming request.
However, this will lead to the undesirable behavior on streaming
requests because WebResponse stream `buffers' messages and there are
no ways to determine how many messages `buffered'.

This commit adds UseCompressionOnStreaming option [default false]
to make streaming behavior desirable and predictable.
@a4lg

This comment has been minimized.

Show comment
Hide comment
@a4lg

a4lg Aug 28, 2014

Contributor

原因が判明したのと、根本的な対処が不可能であることが分かったため再度 open します。
なお、提案ブランチは当初 pull request のコミットからバグ修正とコミットメッセージの実情に合わせた修正を行ったものに差し替えられています (commit 4e6a34e)。

本当の原因は、WebResponse の stream (DeflateStream) が設計からして読み取ったデータのバッファリングを前提としていることでした。また少なくとも Mono においてはこのバッファのサイズ (4KiB) がハードコーディングされているため、ユーザー側からの変更が不可能です。

そのため、コードはほぼ当初の通り (Clone のみ適切に動作するよう変更)、コミットメッセージを本当の原因に合わせて修正したものに関して pull request を再度 open します。

Contributor

a4lg commented Aug 28, 2014

原因が判明したのと、根本的な対処が不可能であることが分かったため再度 open します。
なお、提案ブランチは当初 pull request のコミットからバグ修正とコミットメッセージの実情に合わせた修正を行ったものに差し替えられています (commit 4e6a34e)。

本当の原因は、WebResponse の stream (DeflateStream) が設計からして読み取ったデータのバッファリングを前提としていることでした。また少なくとも Mono においてはこのバッファのサイズ (4KiB) がハードコーディングされているため、ユーザー側からの変更が不可能です。

そのため、コードはほぼ当初の通り (Clone のみ適切に動作するよう変更)、コミットメッセージを本当の原因に合わせて修正したものに関して pull request を再度 open します。

@a4lg a4lg reopened this Aug 28, 2014

azyobuzin added a commit that referenced this pull request Aug 28, 2014

Merge pull request #83 from a4lg/feature/streaming-compression-switch
Add UseCompressionOnStreaming parameter

@azyobuzin azyobuzin merged commit f8774d9 into CoreTweet:master Aug 28, 2014

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci The Travis CI build passed
Details

@a4lg a4lg deleted the a4lg:feature/streaming-compression-switch branch Aug 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment