feat(csharp/src/Drivers/BigQuery): support max stream count setting when creating read session #2289
Conversation
qifanzhang-ms
requested review from
lidavidm and
CurtHagenlocher
as code owners
|
Thanks @qifanzhang-ms Can you update the readme with the new parameter? And also update |
| @@ -35,6 +35,7 @@ public class BigQueryParameters | |||
| public const string Scopes = "adbc.bigquery.scopes"; | |||
| public const string IncludeConstraintsWithGetObjects = "adbc.bigquery.include_constraints_getobjects"; | |||
| public const string GetQueryResultsOptionsTimeoutMinutes = "adbc.bigquery.get_query_results_options.timeout"; | |||
| public const string CreateReadSessionMaxStreamCount = "adbc.bigquery.create_read_session.max_stream_count"; | |||
There was a problem hiding this comment.
Can we name this something a little more meaningful to the user like adbc.bigquery.max_fetch_concurrency?
There was a problem hiding this comment.
Could you please change the constant name to match the property name?
There was a problem hiding this comment.
OK, thanks for the correction.
| @@ -103,8 +103,19 @@ public override QueryResult ExecuteQuery() | |||
|
|
|||
| string table = $"projects/{results.TableReference.ProjectId}/datasets/{results.TableReference.DatasetId}/tables/{results.TableReference.TableId}"; | |||
|
|
|||
| int maxStreamCount = 1; | |||
There was a problem hiding this comment.
Is 1 really the best default value here? (I don't know how we ended up with 1; the documentation clearly recommends 0.
There was a problem hiding this comment.
The default value is set to 1, mainly due to two reasons:
1 The original driver setting is 1. The GBQ MSDI driver is now being used by a large number of CXs, and changing this setting may cause potential regression.
2 The doc does say: If unset or zero, the server will provide a value of streams so as to produce reasonable throughput. But I found in my test that this is not 100% accurate. At least in my test of reading the same table, it took 42.5 minutes when max_stream_count=1 and 46.4 minutes when max_stream_count=0. Although this may be due to other reasons such as the test table content or network fluctuations, it at least shows that potential regression on perf exists.
No description provided.