Re: Race condition in OnHeapHnswGraph
Viliam ġurina <[email protected]> Fri, 16 Jan 2026 09:41:25 +0100
| Newsgroups | gmane.comp.jakarta.lucene.user |
|---|---|
| Message-ID | <CAO=iB8KTVk9mXxBP_CEzLsqPzG_5bqyujydh9Xbke+sqJrEs_w@mail.gmail.com> |
--000000000000a2487f06487d50a3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable I'd like to do the PR, but could you please confirm to me that it's safe to use non-volatile field? I believe this should be the case. Viliam On Thu, Jan 15, 2026 at 12:25=E2=80=AFAM Michael Sokolov <[email protected]= m> wrote: > yeah this looks silly: do you want to open a PR to fix? > > On Mon, Jan 12, 2026 at 6:39=E2=80=AFAM Viliam =C4=8Eurina <viliam.durina= @gmail.com> > wrote: > > > > Hi all, > > > > I'm looking at `OnHeapHnswGraph` code and noticed that the volatile fie= ld > > `graphRamBytesUsed` is modified in `addNode` in a racy way: > > > > long bytesUsed =3D graphRamBytesUsed; > > graphRamBytesUsed =3D bytesUsed + l; > > > > This is equivalent to `graphRamBytesUsed +=3D l`. > > > > This code is susceptible to lost update due to non-atomic > read-modify-write > > operation. > > > > I guess the it's not really a problem, because this code is in fact > > single-threaded when documents are added to the index. It might be > > concurrent during merging, but then `ramBytesUsed()` isn't called, and > it's > > a wasted work. > > > > If the above assumption is correct, then this field should not be > volatile > > to improve performance. If it's not, it should be replaced with > > `AtomicLong` or `LongAdder`. > > > > Viliam > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > --000000000000a2487f06487d50a3--