Skip to content
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

Set last-access-time for only LRU records #18915

Merged
merged 2 commits into from Jun 23, 2021

Conversation

ahmetmircik
Copy link
Member

follow-up of #18624

@hz-devops-test
Copy link

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   ExpirationTimeTest.testLastAccessTime_equals_creationTime_afterFirstPut:375 expected:<1623841580000> but was:<0>
[INFO] 
[ERROR] Tests run: 36348, Failures: 1, Errors: 0, Skipped: 988
[INFO] 

[ERROR] There are test failures.

@mmedenjak mmedenjak added the Source: Internal PR or issue was opened by an employee label Jun 17, 2021
@ufukyilmaz
Copy link
Contributor

ufukyilmaz commented Jun 23, 2021

Why do we need to set the last access time to a different value from the creation time (why do we need to distinguish them)? Most likely, this is because of some existing optimization in the record impl, but I couldn't understand it.

@ufukyilmaz ufukyilmaz self-requested a review June 23, 2021 08:47
@ahmetmircik
Copy link
Member Author

@ufukyilmaz it is to distinguish last-access-time from creation-time. Creating a record doesn't mean it has been accessed. We want to keep this distinction not to break backward behavioral compatibility.

@ahmetmircik ahmetmircik merged commit 74a2f5d into hazelcast:4.2.z Jun 23, 2021
@ahmetmircik ahmetmircik deleted the 4.2/fix/lruImpl2 branch June 23, 2021 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants