エラー型に anyhow クレートを利用 + 非同期実行の構造を大幅に整理 #11
Conversation
| pub struct Config { | ||
| pub database_url: String, | ||
| } | ||
|
|
||
| pub fn load_config() -> Result<Config> { | ||
| let database_url = dotenv::var("DATABASE_URL")?; | ||
| Ok(Config { database_url }) | ||
| } |
There was a problem hiding this comment.
設定値は個別に持ち回すより構造体に入れておいたほうが便利なので、そのようにしてみました
| pub async fn new_pool(config: &Config) -> Result<DbPool> { | ||
| let pool = MySqlPool::connect(&config.database_url).await?; |
There was a problem hiding this comment.
元のコードではここで dotenv::ok(); してから env::var で環境変数を読み取っていましたが、.env からの読み取りは config.rs に移行し、Config 構造体を引数として渡すようにしました。（副作用が少なくなって、扱いやすくなるはず）
There was a problem hiding this comment.
そのコードと直接関係のある話ではないんですけど、MySqlPool の使い方ってそれで合っていますかね・・？
sqlx のドキュメントをみてもよくわからなかったのでとりあえずそのようにしたのですが、正しい使い方なのかどうかが分かっておらずでした・・。
src/main.rs
Outdated
|
|
||
| sleep(Duration::from_secs(329329)); | ||
| break; | ||
| delay_for(Duration::from_secs(329329)).await; |
There was a problem hiding this comment.
とりあえず sleep は数値はそのままにしておいて tokio が提供している関数を使うようにだけ修正しました。
There was a problem hiding this comment.
実行しているコンテナのどれかが終わりしだい、新しい提出を実行するというような実装にすることは可能でしょうか？多分そうとなると本格的にキューなどを実装したほうがよさそうですが・・。
There was a problem hiding this comment.
「コンテナの実行」というのは container_create から始まって container_remove が完了するまで、という認識であってますかね？
合っているとすると、一応今の僕の実装で、「2つのコンテナを実行して、両方ともが完了するまで待つ。両方とも完了したら、次の提出を処理する」みたいな感じになっています。
ただこれだとまだ無駄があって、2つが両方とも完了するまで待つので、片方が1900ms、もう片方が2msで終わるようなコードだった場合に、1900msのほうが終わるまで待っていることになります。
このあたりをさらに無駄なくいい感じにするためには、キューとか、channelとかを駆使する必要がある気がします。
（データベースとの兼ね合いもあるので、慎重に設計する必要がありそう・・・）
There was a problem hiding this comment.
同時実行数を可変にしたいという話と複数コンテナ実行を考えると
将来的には実行スレッド数を設定ファイル側に外出しして、スレッドごとにSQL 発行してlimit 1 がいいのかなあと考えました。
当方複数スレッドの処理わかりません
その場合でもやはり排他制御の問題が出てくると思うのですが、submit テーブルの status をjudge 中の文言に変える等で対応できるかなあと考えました
(アプリ外でキューを持つとなると、複数コンテナ想定ではキュー追加用のアプリケーションの分離、または排他制御がひつようになりそうなので)
| #[async_trait] | ||
| pub trait SubmitRepository { | ||
| async fn get_submits(&self) -> Result<Vec<Submit>>; | ||
| } | ||
|
|
||
| #[async_trait] | ||
| impl SubmitRepository for DbPool { | ||
| async fn get_submits(&self) -> Result<Vec<Submit>> { | ||
| let submits = sqlx::query_as( | ||
| r#" | ||
| SELECT * FROM submits | ||
| WHERE status = 'WJ' OR status = 'WR' AND deleted_at IS NULL | ||
| ORDER BY updated_at ASC | ||
| LIMIT 2 | ||
| "#, | ||
| ) | ||
| .fetch_all(self) | ||
| .await?; | ||
|
|
||
| Ok(submits) | ||
| } | ||
| } |
There was a problem hiding this comment.
Submitをいじくり回す部分をトレイトにすることで、将来的にテストがしやすくなることを期待しています
There was a problem hiding this comment.
なるほど、トレイトってそんな感じで使うんですね・・。勉強になります。
magurotuna
changed the title
|
@cafecoder-dev/judge |
|
コード読んだら大丈夫そうです。 |
|
レビューありがとうございます。では merge してしまいます 💪 |
Closes #3
加えて、main関数内でロックをとったりスレッドを立ち上げたりやっていた箇所をきれいにできないかと思い、
DockerにFutureトレイトを実装して、2つの Future の処理をfutures::try_join!で待ち受ける感じにしてみました。全体的な処理の流れ（
container_createしてからcontainer_removeするだけで良いのか？submit のさばき方は DBから全件取得 -> 2件ずつに分割して処理 でいいのか？など）が把握しきれていないので、修正が必要だとは思いますが、全体的な構造としては大きな問題はないと思います。