[CASSANDRA-20074] AbstractCommitLogService#lastSyncedAt initialized with currentTimeMillis() but later compared and updated with System.nanoTime()

• Type: Bug
• Status: Resolved
• Priority: Low
• Resolution: Fixed
• Fix Version/s: 4.0.15, 4.1.8, 5.0.3, 5.1
• Component/s: Local/Commit Log
• Bug Category: Code - Bug - Unclear Impact
• Severity: Low
• Complexity: Low Hanging Fruit
• Discovered By: Code Inspection
• Platform: All
• Since Version: 4.0

Source Control Link

https://github.com/apache/cassandra/commit/d0c984fa32b81f3fe8058daf0fac2a3081775c9b

Test and Documentation Plan

Ci

• Assignee: Dmitry Konstantinov
• Reporter: Dmitry Konstantinov

Authors

Dmitry Konstantinov

Reviewers

Branimir Lambov, Stefan Miklosovic

• Created: 4 days ago 13:12
• Updated: Yesterday 16:12
• Resolved: Yesterday 11:12

Description

Within org.apache.cassandra.db.commitlog.AbstractCommitLogService

we use System.currentMillis()-based time to initialize lastSyncedAt field:

protected volatile long lastSyncedAt = currentTimeMillis();

but later we use clock.now() = System.nanoTime() to compare and update the field value:

long pollStarted = clock.now();
boolean flushToDisk = lastSyncedAt + syncIntervalNanos <= pollStarted || state != NORMAL || syncRequested; 

As of now we are lucky that in the current JDK implementations System.nanoTime() is much bigger than currentTimeMillis() but it is not guaranteed by a specification of the API. If  nanoTime() < currentTimeMillis() then we can stuck without sync of commit log to disk.

So, we need to align the time sources and change the init logic to:

protected volatile long lastSyncedAt = preciseTime.now();

Issue Links

• is duplicated by: CASSANDRA-20073 AbstractCommitLogService#lastSyncedAt initialized with currentTimeMillis() but later compared and updated with System.nanoTime()
• relates to: CASSANDRA-10202 simplify CommitLogSegmentManager
• links to: GitHub Pull Request #3674
• links to: GitHub Pull Request #3681
• links to: GitHub Pull Request #3682