feat(csharp/src/Drivers/Apache/Spark): low latency test cases #1948
Conversation
CurtHagenlocher
changed the title
|
I'm sorry; I seem to have missed this. It looks like there might be some imminent merge conflicts with #2014 (attn @birschick-bq). |
| { | ||
| TExecuteStatementReq executeRequest = new TExecuteStatementReq(this.connection.sessionHandle, this.SqlQuery); | ||
| SetStatementProperties(executeRequest); | ||
| executeRequest.GetDirectResults = new TSparkGetDirectResults(BatchSizeDefault); |
There was a problem hiding this comment.
Set it to the "current" value of BatchSize, in case it's been update through configuration/programmatically.
| executeRequest.GetDirectResults = new TSparkGetDirectResults(BatchSizeDefault); | |
| executeRequest.GetDirectResults = new TSparkGetDirectResults(BatchSize); |
| ThriftHttpTransport transport = new ThriftHttpTransport(httpClient, config); | ||
| _httpClient.BaseAddress = new UriBuilder(Uri.UriSchemeHttps, hostName, -1, path).Uri; | ||
| _httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); | ||
| _httpClient.DefaultRequestHeaders.UserAgent.ParseAdd("SimbaSparkJDBCDriver/2.06.15 Python/PyHive"); |
There was a problem hiding this comment.
I'd update the UserAgent value and leave a comment as to why this needs to be particular constant. Ideally, the ADBC driver agent name could also be added to your whitelist.
| private static readonly char[] Characters = "0123456789abcdef".ToCharArray(); | ||
| private static readonly Random Random = new Random(); | ||
|
|
||
| public string Generate() |
There was a problem hiding this comment.
Needs a more descriptive name
| public string Generate() | |
| public string GenerateTraceId() |
| long count = 0; | ||
| long target = 80; | ||
| Queue<String> log = new Queue<string>(); | ||
| string FileName = $@"c:\temp\hmsmeta\{DateTime.Now.ToString("yyyyMMdd_HHmm")}__{target}.log"; |
There was a problem hiding this comment.
I would add a logFolder property into the configuration file properties. This is not likely portable or desirable.
| for (int i = 0; i < 32; i++) | ||
| { | ||
| result.Append(Characters[Random.Next(Characters.Length)]); | ||
| } |
There was a problem hiding this comment.
This could be extracted to a function with the length as the parameter and then resused for lines 315-318.
| @@ -32,6 +32,17 @@ internal SparkStatement(SparkConnection connection) | |||
| : base(connection) | |||
| { | |||
| } | |||
| public override QueryResult ExecuteQuery() | |||
There was a problem hiding this comment.
nit: empty line
| public override QueryResult ExecuteQuery() | |
| public override QueryResult ExecuteQuery() |
| @@ -32,6 +32,17 @@ internal SparkStatement(SparkConnection connection) | |||
| : base(connection) | |||
| { | |||
| } | |||
| public override QueryResult ExecuteQuery() | |||
There was a problem hiding this comment.
Should this override code be applied to ExecuteQueryAsync, too?
| var conn = (connection as SparkConnection); | ||
|
|
||
| if(conn != null) | ||
| { | ||
| conn.ResetTraceId(); | ||
| } |
There was a problem hiding this comment.
More concise?
```suggestion
if (connection is SparkConnection sparkConnection) sparkConnection.ResetTraceId();
| statement.SqlQuery = TestConfiguration.Query; | ||
|
|
||
| QueryResult queryResult = statement.ExecuteQuery(); | ||
| for (int i = 0; i < target; i++) |
There was a problem hiding this comment.
This test could be refactored to handle any test "kernel" (lambda) to wrap the test such that we could capture performance information in a log file. What do you think?
| using AdbcStatement statement = adbcConnection.CreateStatement(); | ||
| statement.SqlQuery = TestConfiguration.Query; | ||
|
|
||
| QueryResult queryResult = statement.ExecuteQuery(); | ||
| Tests.DriverTests.CanExecuteQuery(queryResult, TestConfiguration.ExpectedResultsCount); |
There was a problem hiding this comment.
This could be the lambda fed into the performance logger.
|
sorry, I didn't meant to ask for PR right away, since this is still at initial stage, not sure how the reviewers got added. |
|
You can set it to be a Draft PR |
No description provided.