palantir / atlasdb

Transactional Distributed Database Layer
https://palantir.github.io/atlasdb/
Apache License 2.0
46 stars 7 forks source link

Scrubbing has issues #1697

Open tjwilson90 opened 7 years ago

tjwilson90 commented 7 years ago

1) TransactionManagers.create(...) does not start the background scrub task leaving it to individual applications. This is not hard (txManager.getCleaner().start(txManager)), but I seriously doubt many applications are doing it.

2) When background scrubbing is enabled, it runs off a shared _scrub table that serves as a queue of cells to scrub. Every atlas client will run off this same queue, likely redoing work already done by other clients. It would be nicer to simply have one client do the work, holding a lock to exclude other clients (see CoyoteBackgroundReindexReader in acme/spire for an example of this).

3) AbstractDbWriteTable does an INSERT ... WHERE NOT EXISTS in a spin loop to write the sentinel timestamps during scrub and sweep. When combined with the previous issue, this causes a lot of unique constraint violations and retries. In postgres at least this can be specialized to do an INSERT ... ON CONFLICT DO NOTHING to avoid this. Not sure how to do the equivalent on oracle.

clockfort commented 7 years ago

re: Oracle I believe (once you move to utilize an external outside-the-DB lock as per # 2) you can do this with the MERGE syntax where the second "table" you're merging from is from immediate values supplied in dual ; e.g. merge into _scrub using dual on ( blah ) when not matched then insert values ( foo, bar)

(This is very similar to how one would implement concurrency-unsafe upserts in oracle, except in this case we don't want to do the "up"date part)

(There is also probably other ways to do this, including catching the exception inside the Oracle SQL so we don't have to do it in java)

(That all said, we should just fix this by implementing points # 1 and # 2, so that # 3 isn't even really a problem)

rhero commented 7 years ago

Going to backlog this for now, but will take a closer look if more internal AtlasDB clients want to really do scrubbing or have hard delete needs.