Reduce the impact of block methods on tokio runtime

[fengys1996]

What type of enhancement is this?

Performance

What does the enhancement do?

We have some scenarios where we need to call an asynchronous method in a synchronous method in tokio runtime.

example: https://github.com/GreptimeTeam/greptimedb/blob/develop/src/catalog/src/remote/manager.rs#L532

At present, our solution is to create a new thread every time a synchronous method is called, and then call block_on on this thread to execute the asynchronous method.

There are two problems:

1. Every time a synchronous method is called, a thread will be created. It is too expensive.
2. The execution time of the synchronous method is too long, which may cause the tokio runtime blocking.

There are two ideas:

1. use tokio::task::block_in_place, But it doesn't work with the current_thread runtime.
2. use a thread pool, but only problem one is solved, problem 2 is not resolved.

Implementation challenges

No response

[MichaelScofield]

I think we can completely resolve this issue by not using DataFusion's catalog list

[killme2008]

Looks like there is already an async version of catalog in datafusion

apache/datafusion#3777

apache/datafusion#4607

Maybe we need to upgrade datafusion.

[waynexia]

Maybe we need to upgrade datafusion.

They are already available in our deps

[MichaelScofield]

@killme2008 now datafusion only has async version of getting table. we need other methods to be async too, like iterating tables names.

[killme2008]

@killme2008 now datafusion only has async version of getting table. we need other methods to be async too, like iterating tables names.

Got it, i think we can submit an issue to datafusion for it.

[MichaelScofield]

Well, seeing the discussions in apache/datafusion#3777 , I don’t think the async version of catalog list could be easily done, too many breaking changes to APIs including the major one, SessionContext. I think the best strategy is to get rid of the table management of datafusion. We can just use datafusion as a pure query engine. Btw, datafusion itself also realize this issue, see apache/datafusion#5291

…

2023年4月6日 11:34，dennis zhuang ***@***.***> 写道： @killme2008 <https://github.com/killme2008> now datafusion only has async version of getting table. we need other methods to be async too, like iterating tables names. Got it, i think we can submit an issue to datafusion for it. — Reply to this email directly, view it on GitHub <#1335 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHR2DYCOE63NLHHHWVKVYTW7Y2UHANCNFSM6AAAAAAWUZL7AU>. You are receiving this because you commented.

[evenyag]

We could cache the catalog data and refresh them in the async context. I remember that @v0y4g3r has some ideas on it.

[MichaelScofield]

Ballista shares the same idea, it has a “refresh” method that update all catalogs cached in local. However, the “refresh” is an all-to-all comparison, not efficient. If we were to implement the same refreshment, I think we have to do the same: fetching all catalogs from remote and then compare(or replace) with local cache. Not good.

…

2023年4月6日 12:13，Yingwen ***@***.***> 写道： We could cache the catalog data and refresh them in the async context. I remember that @v0y4g3r <https://github.com/v0y4g3r> has some ideas on it. — Reply to this email directly, view it on GitHub <#1335 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHR2D6GIMCTC4SF5GGO7YLW7Y7FJANCNFSM6AAAAAAWUZL7AU>. You are receiving this because you commented.

[evenyag]

Anyway, a cache is necessary if we don't want to issue a remote call to the meta each time we access the remote catalog. But I think we don't need to do an all-to-all comparison, we only need to replace the local cache with the remote data.

We can use different cache policies for catalogs, schemas, tables, and tables' metadata. e.g.

• For catalogs, schemas, and tables, we update them periodically
• For metadata of tables, we can use a fixed capacity cache (with TTL?) and update the individual table entry on a cache miss

[fengjiachun]

metasrv has push capability, which can achieve incremental updates of cache based on push.