You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@roller.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2026/10/11 02:03:00 UTC
[jira] [Work logged] (ROL-2185) A background search re-index can put a deleted comment back into EclipseLink's shared cache
[ https://issues.apache.org/jira/browse/ROL-2185?focusedWorklogId=1046833&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1046833 ]
ASF GitHub Bot logged work on ROL-2185:
---------------------------------------
Author: ASF GitHub Bot
Created on: 11/Oct/26 02:02
Start Date: 11/Oct/26 02:02
Worklog Time Spent: 10m
Work Description: mraible opened a new pull request, #214:
URL: https://github.com/apache/roller/pull/214
Fixes [ROL-2185](https://issues.apache.org/jira/browse/ROL-2185), which is behind the intermittent `RollerAtomManagementTest.commentModeration` failure (line 419, a deleted comment is still found). It failed on CI for #160 and locally on #155.
Approving or deleting a comment queues a search re-index of its entry, which runs on a background thread and loads the entry's approved comments. If that read overlaps a comment being deleted, EclipseLink can put the deleted comment back into its shared cache after the delete commits. `getComment(id)` uses `EntityManager.find`, which is answered from that cache, so it returns the deleted comment even though the row is gone. List pages and feeds use JPQL queries and aren't affected.
The fix sets `eclipselink.cache.shared.WeblogEntryComment=false` in `persistence.xml`, so comments are always read from the database. Every other entity keeps the shared cache.
How I verified it:
- A throwaway stress test (attached to the JIRA ticket) creates a comment, queues a re-index of its entry, deletes the comment and looks it up. On master the deleted comment came back 128 times out of 1500, with the database row gone every time, and 0 times out of 1500 with the re-index turned off. With this change it's 0 out of 1500 with the re-index on.
- The new `CommentSharedCacheTest` checks that a loaded comment isn't in the shared cache while a weblog entry still is. It fails without the `persistence.xml` change and passes with it.
- The full build passes (492 tests), and so does `apache-rat:check`.
The cost is that loading a comment by id now always goes to the database. Comments are mostly read through queries anyway, so I don't expect a noticeable difference.
I left `CHANGES.md` alone because master's 7.0.0 section doesn't list bug fixes yet. Happy to add a line if we want one. The same race probably exists on `roller-6.1.x`, but I've only reproduced it on master.
Issue Time Tracking
-------------------
Worklog Id: (was: 1046833)
Remaining Estimate: 0h
Time Spent: 10m
> A background search re-index can put a deleted comment back into EclipseLink's shared cache
> -------------------------------------------------------------------------------------------
>
> Key: ROL-2185
> URL: https://issues.apache.org/jira/browse/ROL-2185
> Project: Apache Roller
> Issue Type: Bug
> Components: Data Model & JPA Backend
> Affects Versions: 6.1.2
> Reporter: Matt Raible
> Priority: Minor
> Attachments: StaleCommentRaceScratchTest.java
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> RollerAtomManagementTest.commentModeration fails intermittently on master at line 419, where it checks that a comment deleted through AtomPub is gone:
> {code}
> RollerAtomManagementTest.commentModeration:419 expected: <null> but was: <{5657e458-..., test, test, 2026-10-11 00:30:30.253}>
> {code}
> It failed once in a local full-suite run and once on CI (JDK 21) for PR #160, and passes when re-run. The test isn't wrong. It exposes a race in Roller itself.
> What happens:
> 1. Approving a comment (CommentCollection.putEntry) and deleting one (deleteEntry) both call IndexManager.addEntryReIndexOperation(entry).
> 2. LuceneIndexManager.scheduleIndexOperation runs that ReIndexEntryOperation on a background thread. To build the Lucene document it re-reads the entry and loads its approved comments (IndexOperation.getDocument -> WeblogEntry.getComments()).
> 3. If that background read fetches a comment's row just before another thread deletes the comment, EclipseLink can register the object in its shared (L2) cache after the delete has committed and evicted it.
> 4. A later WeblogEntryManager.getComment(id) uses EntityManager.find, which is answered from the shared cache, so it returns the deleted comment even though the row is gone.
> In commentModeration, approving the pending comment schedules a re-index that loads the "approved" comment, which the test deletes right afterwards.
> Reproduction:
> A throwaway test on master (359c4804c) loops: create an approved comment, call addEntryReIndexOperation for its entry, wait 0-4 ms, delete the comment and flush, wait 30 ms, then call getComment(id) and also count the row with JPQL.
> {code}
> with the re-index: getComment() returned the deleted comment 128/1500 times; DB row present 0 times
> without the re-index: getComment() returned the deleted comment 0/1500 times; DB row present 0 times
> {code}
> So the database is correct and only the shared cache is stale, and only when a background re-index overlaps the delete. I can attach the test.
> Impact:
> Any find-by-id of a comment (WeblogEntryManager.getComment(String)) can return a comment that was deleted while a re-index of the same entry was running, until the cache entry is evicted. List pages and feeds use JPQL queries that read the database, so they aren't affected. The same pattern may apply to other entities that background index operations read; I only measured comments.
> Possible fixes (not yet tried):
> - Turn off the shared cache for WeblogEntryComment, e.g. eclipselink.cache.shared.WeblogEntryComment=false or <cacheable>false</cacheable> in its orm.xml. Simplest, at some read cost.
> - Keep the background index operations from populating the shared cache, e.g. with a query hint on the reads they do. This needs checking against EclipseLink's documentation.
> - Test-only workaround: wait for queued background index operations before deleting in commentModeration. This would hide the race rather than fix it.
> Whichever fix is chosen, the stress test above makes it easy to verify (it should report 0 with the re-index enabled).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)