Yingjianw/list db with follower reader timestamp #589
Conversation
stevie9868
force-pushed
the
yingjianw/ListDBWithFollowerReaderTimestamp
branch
3 times, most recently
from
5e2624a to
acc623e
Compare
|
To make things cleaner, could you first separate out the revert changes from the core logic changes? |
stevie9868
force-pushed
the
yingjianw/ListDBWithFollowerReaderTimestamp
branch
from
acc623e to
289adee
Compare
| @@ -77,6 +77,7 @@ public void create(final ConnectorRequestContext context, final DatabaseInfo dat | |||
| * {@inheritDoc}. | |||
| */ | |||
| @Override | |||
| @Transactional | |||
There was a problem hiding this comment.
Why did you add this annotation to this method, if it's for the exists check, the exists check and deletion do not need to be transactional. If it's for something else then what is it for?
There was a problem hiding this comment.
Good Catch. Not sure why I added this, might be something I accidentally included during my rebase...
| @@ -226,6 +226,7 @@ class MetacatFunctionalSpec extends Specification { | |||
|
|
|||
| when: | |||
| api.createDatabase(catalog.name, databaseName, dto) | |||
| Thread.sleep(5000) | |||
There was a problem hiding this comment.
In the other tests the wait time is 10 seconds delay is this delay enough for the follower read?
There was a problem hiding this comment.
yeah, it turns out 5 seconds is enough. I will adjust other test to use 5 seconds as well.
| .map(d -> QualifiedName.ofDatabase(name.getCatalogName(), d.getDbName())) | ||
| final String dbPrefix = prefix == null ? "" : prefix.getDatabaseName(); | ||
| final List<QualifiedName> qualifiedNames = polarisStoreService.getDatabaseNames( | ||
| dbPrefix, sort, 1000) |
There was a problem hiding this comment.
Should we try a larger page size like 5000 or 10000? That's what we bumped it to for table names and it seems it could be more efficient.
There was a problem hiding this comment.
Good point.
I think 1000 was also too small, and once we did a backfill, we need to get the result back and forth from the db, which increase the time holding to the db connection.
stevie9868
force-pushed
the
yingjianw/ListDBWithFollowerReaderTimestamp
branch
from
f739e60 to
73210f5
Compare
* follower_read_timestamp for list db calls * address comments --------- Co-authored-by: Yingjian Wu <[email protected]>
As we see more dbs added to our metadata service, the load on crdb increases when there are many listDB calls, and we see only subset of certain nodes in our cluster experiencing high cpu usage.
Thus, in order to better spread the traffic, for list dbs operation, we are going to use the read_follower_timestamp.
Unfortunately, we need to rollback previous 2 commits as we don't want them to be included into the same rollout