Skip to content

Commit 2461e19

Browse files
author
Hayim.Shaul@ibm.com
committed
fix
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
1 parent 00c2023 commit 2461e19

6 files changed

Lines changed: 25 additions & 12 deletions

File tree

docs/imgs/storage_db.puml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ package "Token Store (TokenDB)" {
127127
* idx : INT <<PK, NOT NULL, FK>>
128128
--
129129
consumer_tx_id : TEXT <<NOT NULL>>
130-
created_at : TIMESTAMP <<NOT NULL>>
130+
created_at : TIMESTAMPTZ <<NOT NULL>>
131131
}
132132

133133
entity "TokenSKICleanups" as tkn_ski_cleanups {

docs/services/selector.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,11 @@ Two properties of the pass are worth spelling out:
140140
deleted; the other outputs of the same transaction keep their locks.
141141

142142
The pass is the same statement on every SQL backend, so lock lifetime does not depend on
143-
whether a deployment runs on SQLite, the in-memory backend, or Postgres. On Postgres a
144-
single replica per TMS runs it per tick, elected through an advisory lock; SQLite is
145-
non-distributed and always runs it locally.
143+
whether a deployment runs on SQLite, the in-memory backend, or Postgres. `created_at` is
144+
stored as `TIMESTAMPTZ`, so the comparison with the database-side `NOW()` expression is
145+
always timezone-consistent on Postgres regardless of the session `TimeZone` setting.
146+
On Postgres a single replica per TMS runs the pass per tick, elected through an advisory
147+
lock; SQLite is non-distributed and always runs it locally.
146148

147149
### In-Memory Locker Internals
148150

token/services/storage/db/driver/token.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,14 @@ type TokenLockStore interface {
339339
Lock(ctx context.Context, tokenID *token.ID, consumerTxID transaction.ID, walletID string) error
340340
// UnlockByTxID unlocks all tokens locked by the consumer TX
341341
UnlockByTxID(ctx context.Context, consumerTxID transaction.ID) error
342-
// Cleanup removes the locks such that either:
343-
// 1. The transaction that locked that token is valid or invalid;
344-
// 2. The lock is too old.
342+
// Cleanup removes stale token locks. A lock is stale when either:
343+
// 1. The *consuming* transaction (the one trying to spend the token,
344+
// identified by consumer_tx_id) has reached a terminal failure status
345+
// (Deleted or Orphan); or
346+
// 2. The lock is older than leaseExpiry (covers consumers that crashed
347+
// before reaching any terminal status).
348+
// The producer transaction (the one that created the token, identified by
349+
// (tx_id, idx)) is irrelevant to expiry; see #2018.
345350
Cleanup(ctx context.Context, leaseExpiry time.Duration) error
346351
// AcquireCleanupLeadership attempts to acquire leadership for the
347352
// cleanup tick, so only one replica runs Cleanup per tick. Non-distributed

token/services/storage/db/sql/common/tokenlock.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func (db *TokenLockStore) GetSchema() string {
9595
tx_id TEXT NOT NULL,
9696
idx INT NOT NULL,
9797
consumer_tx_id TEXT NOT NULL,
98-
created_at TIMESTAMP NOT NULL,
98+
created_at TIMESTAMPTZ NOT NULL,
9999
PRIMARY KEY(tx_id, idx),
100100
FOREIGN KEY (tx_id, idx) REFERENCES %s
101101
);
@@ -125,7 +125,10 @@ func IsExpiredToken(tokenRequests, tokenLocks common3.Table, leaseExpiry time.Du
125125
// whether the lock is still live. The condition is correlated rather than a
126126
// partial-key IN on tx_id, so cleanup removes only the matching (tx_id, idx) rows
127127
// and leaves the other indices of the same transaction locked. See #2018.
128-
func IsStaleLock(tokenLocks, tokenRequests common3.Table, leaseExpiry time.Duration) cond.Condition {
128+
//
129+
// The argument order matches IsExpiredToken (tokenRequests first, tokenLocks second)
130+
// so a swapped call is immediately visible and consistent across this file.
131+
func IsStaleLock(tokenRequests, tokenLocks common3.Table, leaseExpiry time.Duration) cond.Condition {
129132
return cond.Or(
130133
cond.OlderThan(tokenLocks.Field("created_at"), leaseExpiry),
131134
cond.Exists(
@@ -143,12 +146,13 @@ func IsStaleLock(tokenLocks, tokenRequests common3.Table, leaseExpiry time.Durat
143146
// Cleanup releases the stale token locks: those whose consuming transaction is
144147
// Deleted or Orphan, and those whose lease is older than leaseExpiry. Only the
145148
// affected (tx_id, idx) rows are deleted. The same statement is used by every SQL
146-
// backend, so lock lifetime does not depend on the driver in use.
149+
// backend. created_at is declared TIMESTAMPTZ so the comparison with the
150+
// database-side NOW() expression is always timezone-consistent on Postgres.
147151
func (db *TokenLockStore) Cleanup(ctx context.Context, leaseExpiry time.Duration) error {
148152
tokenLocks, tokenRequests := q.Table(db.Table.TokenLocks), q.Table(db.Table.Requests)
149153

150154
query, args := q.DeleteFrom(db.Table.TokenLocks).
151-
Where(IsStaleLock(tokenLocks, tokenRequests, leaseExpiry)).
155+
Where(IsStaleLock(tokenRequests, tokenLocks, leaseExpiry)).
152156
Format(db.ci)
153157

154158
db.Logger.Debug(query, args)

token/services/storage/db/sql/postgres/tokenlock.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ func (db *TokenLockStore) Cleanup(ctx context.Context, leaseExpiry time.Duration
9797
}
9898

9999
// logStaleLocks logs the token locks that are about to be deleted.
100+
// NOW() returns timestamptz; created_at is also TIMESTAMPTZ, so both sides of
101+
// the age comparison are timezone-consistent.
100102
func (db *TokenLockStore) logStaleLocks(ctx context.Context, leaseExpiry time.Duration) error {
101103
if !db.Logger.IsEnabledFor(zapcore.InfoLevel) {
102104
return nil

token/services/storage/db/sql/sqlite/tokenlock_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func TestCleanupSQLShape(t *testing.T) {
4444

4545
tokenLocks, requests := q.Table("TokenLocks"), q.Table("Requests")
4646
query, args := q.DeleteFrom("TokenLocks").
47-
Where(common3.IsStaleLock(tokenLocks, requests, 5*time.Second)).
47+
Where(common3.IsStaleLock(requests, tokenLocks, 5*time.Second)).
4848
Format(NewConditionInterpreter())
4949

5050
Expect(query).To(ContainSubstring("Requests.tx_id = TokenLocks.consumer_tx_id"))

0 commit comments

Comments
 (0)