Skip to content
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

ポーリング対象範囲の改善と伴う変数の追加 #133

Merged
merged 13 commits into from
Jan 29, 2024

Conversation

alter334
Copy link
Contributor

@alter334 alter334 commented Jan 24, 2024

messagePollar周りの改善案です
goルーチンに対する理解が浅いため,並行処理でちゃんと動くか把握できてないので問題があれば教えてください

やってみたこと

メッセージポーリング範囲を1分追加改善

  • SearchMessagesAPIが直近数十秒の投稿を拾うか否か不安定なため(取得もれもここで起きているものがあると考えられる)
  • この変更以外が現在の仕様の場合3分ごとに直近4分の投稿を拾う仕様になる
  • メッセージ収集のfromを前回検索時の最新メッセージの投稿日時にした

messageProcessorにlastCheckMessage(string),tmplastCheckMessage(string)を追加&それに対応した既存メソッドの変更

  • 前回のポーリングで取得した最新メッセージのUUIDを格納(lastCheck...)
  • 先の仕様変更によりポーリング対象が広くなっているため重複してDM送られる可能性がある
  • そのため、前回集計で扱ったメッセージが現れた場合にメッセージ読み取りを終了するようにした(これにより該当メッセージが削除されない限り重複したDMは発生しない...筈)
  • tmplast... については並行処理を考慮して設定(そもそも並行処理に対して知識が浅いので上手くいくか分からないです)

@alter334 alter334 marked this pull request as draft January 24, 2024 15:02
@alter334 alter334 marked this pull request as ready for review January 24, 2024 15:02
Copy link
Collaborator

@Pugma Pugma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

パッと見だとこれくらいです

ただUUIDで管理するんじゃなく、前回取得時の最新メッセージの投稿時刻を検索クエリのAfterに入れるっていう案が出ていて、そちらのほうが投稿が削除された場合でも不具合がないのかな、と思っています

server/traqmessage/collect.go Outdated Show resolved Hide resolved
@alter334
Copy link
Contributor Author

最新メッセージの投稿時間を記録しておく方法なら若干ブラックボックス気味な並行処理部分での条件分岐が要らなくなりそうです
試してみます

@alter334
Copy link
Contributor Author

元の実装にかなり近くなった気がします goルーチン中での処理が消えました:iihanashi:

@alter334 alter334 changed the title ポーリング対象範囲を1分追加と伴う変数の追加 ポーリング対象範囲の改善と伴う変数の追加 Jan 25, 2024
Copy link
Collaborator

@Pugma Pugma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最新の投稿時刻を保持するにあたって、DBでテーブルを作っておく必要があると思うのでお願いします
現状だと再起動時は最新時刻を起点に収集するはずで、前回の最新投稿の時間を使えないので収集漏れの可能性があります

@alter334
Copy link
Contributor Author

これ再起動時に何もしてないな(反映させます)

Copy link
Collaborator

@Pugma Pugma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認しました
もう少しだけお願いします!:pray:

server/traqmessage/collect.go Outdated Show resolved Hide resolved
server/traqmessage/collect.go Outdated Show resolved Hide resolved
server/traqmessage/collect.go Show resolved Hide resolved
Copy link
Collaborator

@Pugma Pugma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あとこれも確認よろしく!

server/traqmessage/collect.go Outdated Show resolved Hide resolved
server/traqmessage/collect.go Outdated Show resolved Hide resolved
@alter334
Copy link
Contributor Author

他何か改善点ありましたっけ
あったようななかったような

@Pugma
Copy link
Collaborator

Pugma commented Jan 27, 2024

一旦mainブランチの変更点を取り込んでほしいかも

Copy link
Collaborator

@Pugma Pugma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

問題なさそうです!
改善ありがとうございました!!!

@Pugma Pugma merged commit 819f902 into traP-jp:main Jan 29, 2024
2 checks passed
@alter334 alter334 deleted the enhancepolling branch January 29, 2024 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants