Skip to content

採番時に確保(予約)失敗した場合に確保できていない番号が使用されてしまう問題を修正 #43

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.

Sign up for GitHub

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

Closed
tksugimoto wants to merge 3 commits into from
Closed

採番時に確保(予約)失敗した場合に確保できていない番号が使用されてしまう問題を修正 #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
54132ec
test: 予約に失敗した場合は次の採番要求で再度予約が要求される
tksugimoto Jul 14, 2021
0fbaa7e
fix: 確保できていない採番が成功するバグを修正
tksugimoto Jul 14, 2021
a821301
test: 予約失敗時の番号飛びについてテストに追加
tksugimoto Jul 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lerna-util-sequence/src/main/scala/lerna/util/sequence/SequenceFactoryWorker.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ private[sequence] final class SequenceFactoryWorker(
if (msg.sequenceSubId === sequenceSubId) {
import sequenceContext._

if (nextValue <= maxSequenceValue) {
if (nextValue <= maxSequenceValue && nextValue <= maxReservedValue) {
msg.replyTo ! SequenceGenerated(nextValue, sequenceSubId)
logger.debug("SequenceGenerated when ready: {}", nextValue)
} else {
Expand Down
63 changes: 63 additions & 0 deletions lerna-util-sequence/src/test/scala/lerna/util/sequence/SequenceFactoryWorkerSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,69 @@ class SequenceFactoryWorkerSpec
testKit.stop(worker)
}

"予約に失敗した場合は次の採番要求で再度予約が要求される" in {
val storeProbe = createTestProbe[SequenceStore.Command]()
val firstValue = 3
val incrementStep = 10
val reservationAmount = 2
Copy link
Contributor Author

@tksugimoto tksugimoto Jul 14, 2021

Choose a reason for hiding this comment

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

val reservationAmount = 2

val reservationAmount = 1 の場合に、予約失敗 SequenceStore.ReservationFailed が発生すると、SKIPされる番号がある影響でずっと予約され続けて採番が成功しない問題があった。

2021-07-14 19:48:55.348 INFO    lerna.util.sequence.SequenceFactoryWorker$      akka://SequenceFactoryWorkerSpec/user/$c        -       dummy   Reserving sequence: remain 0, add 1, current max reserved: 3

2021-07-14 19:48:55.462 INFO    lerna.util.sequence.SequenceFactoryWorker$      akka://SequenceFactoryWorkerSpec/user/$c        -       dummy   Reserving sequence: remain 0, add 1, current max reserved: 13

2021-07-14 19:48:55.566 INFO    lerna.util.sequence.SequenceFactoryWorker$      akka://SequenceFactoryWorkerSpec/user/$c        -       dummy   Reserving sequence: remain 0, add 1, current max reserved: 23

2021-07-14 19:48:55.675 INFO    lerna.util.sequence.SequenceFactoryWorker$      akka://SequenceFactoryWorkerSpec/user/$c        -       dummy   Reserving sequence: remain 0, add 1, current max reserved: 33

TODO:
別Issue対応

Copy link
Contributor Author

@tksugimoto tksugimoto Jul 14, 2021

Choose a reason for hiding this comment

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

#44 reservationAmount = 1 での採番で無限ループのリスク · Issue #44 · lerna-stack/lerna-app-library

Copy link
Contributor

@negokaz negokaz Jul 14, 2021

Choose a reason for hiding this comment

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

@tksugimoto
reserving から ready に戻すときに元の nextValue に戻すと問題がありますか？

Copy link
Contributor Author

@tksugimoto tksugimoto Jul 14, 2021

Choose a reason for hiding this comment

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

できそうですが、複雑になるので別のバグの発生を気にして選択しませんでした。

現状、結構複雑なので状態の整理をしたほうが良さそうです。

Copy link
Contributor Author

@tksugimoto tksugimoto Jul 15, 2021

Choose a reason for hiding this comment

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

val reservationAmount = 1 の場合に、予約失敗 SequenceStore.ReservationFailed が発生すると、SKIPされる番号がある影響でずっと予約され続けて採番が成功しない問題があった。

別Issue対応

  1. nextValue <= maxReservedValue の条件により stash
  2. ReservationFailed によって unstash & ready 遷移
  3. 無限ループ

という流れなのでこのPRの修正が原因(の一つ)だった

val worker = spawn(
SequenceFactoryWorker
.apply(
maxSequenceValue = 999,
firstValue = firstValue,
incrementStep = incrementStep,
reservationAmount = reservationAmount,
storeProbe.ref,
idleTimeout = 10.seconds,
sequenceSubId,
),
)
// 初期化
inside(storeProbe.receiveMessage()) {
case result: SequenceStore.InitialReserveSequence =>
result.replyTo ! SequenceStore.InitialSequenceReserved(
initialValue = firstValue, // 3
maxReservedValue = firstValue + (incrementStep * (reservationAmount - 1)),// 13
)
}

// 採番要求: reservationAmount が 2 なので、2回採番すると枯渇 → 予約
{
val replyToProbe = createTestProbe[SequenceFactoryWorker.SequenceGenerated]()
worker ! SequenceFactoryWorker.GenerateSequence(sequenceSubId, replyToProbe.ref)
expect(replyToProbe.receiveMessage().value === BigInt(3)) // firstValue
worker ! SequenceFactoryWorker.GenerateSequence(sequenceSubId, replyToProbe.ref)
expect(replyToProbe.receiveMessage().value === BigInt(13)) // firstValue + incrementStep
}

// 予約を失敗させる
inside(storeProbe.receiveMessage()) {
case result: SequenceStore.ReserveSequence =>
result.replyTo ! SequenceStore.ReservationFailed
}

// 採番要求: 予約できていないのでレスポンスは来ない
val replyToProbe = createTestProbe[SequenceFactoryWorker.SequenceGenerated]()
worker ! SequenceFactoryWorker.GenerateSequence(sequenceSubId, replyToProbe.ref)
replyToProbe.expectNoMessage()

// 再度予約が要求される
inside(storeProbe.receiveMessage()) {
case result: SequenceStore.ReserveSequence =>
expect(result.maxReservedValue === BigInt(firstValue + incrementStep)) // 13
expect(result.reservationAmount === reservationAmount)
expect(result.sequenceSubId === sequenceSubId)
result.replyTo ! SequenceStore.SequenceReserved(
maxReservedValue = result.maxReservedValue + incrementStep * reservationAmount,
)
}

// ※ 現在の実装だと 23 は skip されて歯抜けになる
expect(replyToProbe.receiveMessage().value === BigInt(33)) // firstValue + incrementStep * 3

testKit.stop(worker)
}

"初期化時に上限を超えた場合はリセットする" in {
val storeProbe = createTestProbe[SequenceStore.Command]()
val maxSequenceValue = 999
Expand Down