[scd] Don't disable lock with timeBasedNotificationIndex on yugabyte

[the-glu]

Disabling lock with yugabyte result in prober test not passing dues to performance. This enable it back only for yugabyte.

[BenjaminPelletier]

This seems very surprising. Isn't the purpose of locking subscriptions to serialize contention between modification of their notification indices? If their notification indices aren't being modified (because of the use of the time-based notification index), why would performance suffer in Yugabyte when we didn't lock the subscriptions while using the time-based notification index? I'm confused as to what mechanism would lead to increased performance in Yugabyte when adding a lock on subscriptions when the time-based notification index was already in use.

[the-glu]

We do have fewer datapoints/investigations on Yugabyte than CockroachDB, as Cockroach was the focus, but based on what I know:

• The subscriptions table still needs to be read, which means that if one query adds a subscription, we still have a form of contention (hence the less-than-expected improvements on CockroachDB with the flag)
• Yugabyte seems to perform worse than CockroachDB under contention, as of now. I had to fix a few things because we didn't pass performance tests with Yugabyte:
  • [yugabyte] Implement Transact retries #1570 to implement retries, as Yugabyte doesn't automatically retry queries
  • [rid] Improve UpdateNotificationIdxsInCells by locking row in order #1571 because locking mechanisms seem to be less smart

I haven't performed a deep dive into Yugabyte mechanisms, but in general, it seems we need to lock elements correctly to avoid contention performance issues, whereas CockroachDB automatically handles some of these optimizations.