Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: Extend the ODBC metric by differentiating between 32 and 64bit platforms #36753

Merged
merged 5 commits into from Dec 18, 2018

Conversation

Projects
None yet
5 participants
@astefan
Copy link
Contributor

commented Dec 18, 2018

At the moment, ODBC requests are tracked in one metric only. This PR addresses #36740 by differentiating between 32bit and 64bit platforms in two different metrics. Also, the "overall" ODBC metric will now track a total of requests between 32 and 64bit ODBC metrics.
We may extend this in the future with even a more granular set of metrics specific to ODBC (accounting for Windows version as well).

@elasticmachine

This comment has been minimized.

Copy link

commented Dec 18, 2018

@astefan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

run the gradle build tests 1
run the gradle build tests 2

@costin

costin approved these changes Dec 18, 2018

Copy link
Member

left a comment

LGTM

astefan added some commits Dec 18, 2018

@astefan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

run the gradle build tests 1
run the gradle build tests 2

@matriv
Copy link
Contributor

left a comment

LGTM, left minor comment

public static final List<String> CLIENT_IDS = Arrays.asList(CLI, CANVAS);
public static final String ODBC_32 = "odbc32";
private static final String ODBC_64 = "odbc64";
public static final List<String> CLIENT_IDS = Arrays.asList(CLI, CANVAS, ODBC_32, ODBC_64);

This comment has been minimized.

Copy link
@matriv

matriv Dec 18, 2018

Contributor

Really minor: This and below can be Immutable sets, so the contains() we call on them are faster.
Not an issue for such small collections, but more future-proof and since we only use them for the contains() makes more sense to be sets instead of lists.

@astefan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

run the gradle build tests 1
run the gradle build tests 2

@astefan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

@elasticmachine run default distro tests

@astefan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

run the gradle build tests 2

@matriv

matriv approved these changes Dec 18, 2018

@astefan astefan merged commit 4bc9bff into elastic:master Dec 18, 2018

7 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci-1 Build finished.
Details
elasticsearch-ci-2 Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docbldesx Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details

@astefan astefan deleted the astefan:36740_fix branch Dec 18, 2018

astefan added a commit that referenced this pull request Dec 18, 2018

SQL: Extend the ODBC metric by differentiating between 32 and 64bit p…
…latforms (#36753)

* The "overall" ODBC metric will now track a total of requests between 32bit and 64bit ODBC metrics, being calculated passively, upon request.

(cherry picked from commit 4bc9bff)

@colings86 colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.