-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update pgx to v4 #26
Update pgx to v4 #26
Conversation
jobsManaged map[int64]*Job | ||
// TODO: add a way to specify default queueing options | ||
} | ||
|
||
// NewClient2 creates a new Client that uses the pgx pool. | ||
func NewClient2(pool *sql.DB) (*Client, error) { |
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.
今回のメジャーバージョンアップデートを機にNewClientを1つに統合しました。
@@ -275,28 +249,6 @@ func (c *Client) dischargeJob(j *Job) { | |||
delete(c.jobsManaged, j.ID) | |||
} | |||
|
|||
// type bytea []byte |
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.
不要なコメントアウト行は削除します。
jobsManaged map[int64]*Job | ||
// TODO: add a way to specify default queueing options | ||
} | ||
|
||
// NewClient2 creates a new Client that uses the pgx pool. | ||
func NewClient2(pool *sql.DB) (*Client, error) { | ||
stmtJobStats, err := pool.Prepare(sqlJobStats) |
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.
こちらはprepared statementとする必要はなく、単にクエリ発行すればいいだけのものだったので↓でクエリ発行するようにしています。
https://github.com/kanmu/qg/pull/26/files#diff-709e128a0c68e210d698775e3bff0a9c801a201e19d55871770d61ba0797f2e9L410
@@ -321,16 +273,16 @@ var ErrAgain = errors.New("maximum number of LockJob attempts reached") | |||
// | |||
// After the Job has been worked, you must call either Done() or Error() on it | |||
// in order to return the database connection to the pool and remove the lock. | |||
func (c *Client) LockJob(queue string) (*Job, error) { | |||
conn, err := stdlib.AcquireConn(c.pool) |
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.
connection取得関数は今後stdlibを使うのではなくpool.Acquireを使う感じになります。
return nil, err | ||
} | ||
} | ||
stdlib.ReleaseConn(c.pool, conn) //nolint:errcheck | ||
|
||
conn.Release() |
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.
これ、上でdefer conn.Release() すれば良さそう?と思いました。
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.
defer だと L:318-L:319 の return 時にも conn を Release することになります。その場合 pool に戻された conn が別のワーカーに使われてしまうため、既存実装では Release しないよう設計されていると理解していました。
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.
↓の部分でReleaseしないのは意図的っぽいのでそのようにはしませんでした。
https://github.com/kanmu/qg/pull/26/files#diff-709e128a0c68e210d698775e3bff0a9c801a201e19d55871770d61ba0797f2e9L367
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.
ああ、じゃあこれでいいですね、、ありがとうございます。
Co-authored-by: Hidetatz Yaginuma <[email protected]>
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.
LGTM
🤗 |
pgxをv4へアップデートしました。
pgxのv4のChangelogは以下の通りです。
https://github.com/jackc/pgx/blob/28ad4873d3817802074177603ff9cefbb4dc9449/CHANGELOG.md
qg内における変更点は主に以下の点です。
このp-rはBreaking Changeを含むためリリース時はメジャーバージョンアップデートを行い、v2.0.0としてリリース予定です。