I think the motivation of acquiring S locks on tuples from disk components is to check whether the key is currently being modified by a transaction, and thus we can use the "latest" version. However, based on our discussion yesterday (me, Taewoo and Mike), this is not a strict requirement, since a key could be update right after we acquire an S lock on the key which we will miss anyway. Thus, it won't provide any stronger guarantees than "read committed".
I've submitted a patch to only acquire S locks for memory components, including both primary index and secondary index search (in case of index-only plan). Best regards, Chen Luo On Wed, May 9, 2018 at 11:10 AM, abdullah alamoudi <[email protected]> wrote: > The main use of instant locks (at least before index only) was to prevent > reading uncommitted records. Hence, we could skip locking when reading from > disk. > Locking in that case doesn't give any better guarantees.. If you think > otherwise, please give an example. > > Can someone (Maybe Taewoo?) give an explanation as to why this was changed? > > Cheers, > Abdullah. > > > On May 8, 2018, at 8:15 PM, Chen Luo <[email protected]> wrote: > > > > Hi Devs, > > > > I recently noticed there is some performance degradation of primary index > > scans after syncing with current master (15% - 20% with record size > > ~300bytes each). It seems this is caused by a recently patch which > requires > > an instant S lock for every record in all components [1]. Acquiring a > lock > > on for each record is relatively expensive, and it becomes even worse for > > smaller records. And I believe previously we've omitted locking for > > scanning disk components intentionally, because records in disk > components > > must have already been committed and cannot change (I assume we're > > implementing something similar to "Read Committed" semantics?) > > > > I propose that this patch needs to be somehow reverted to avoid locking > on > > a record-basis. As for index-only patch, one can safely proceed if a > tuple > > is found from the disk component, again because it has already been > > committed. Thoughts? > > > > Best regards, > > Chen Luo > > > > > > [1] https://asterix-gerrit.ics.uci.edu/#/c/2623/ > >
