You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/10/27 00:41:04 UTC
[GitHub] [druid] techdocsmith opened a new pull request #10532: Document correlation between credential iterations and query latency
techdocsmith opened a new pull request #10532:
URL: https://github.com/apache/druid/pull/10532
### Description
Documents the correlation between credential iterations and query latency. Includes some minor additional stylistic changes.
This PR has:
- [x] added documentation for features or behaviors.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] techdocsmith commented on a change in pull request #10532: Document correlation between credential iterations and query latency
Posted by GitBox <gi...@apache.org>.
techdocsmith commented on a change in pull request #10532:
URL: https://github.com/apache/druid/pull/10532#discussion_r513713688
##########
File path: docs/development/extensions-core/druid-basic-security.md
##########
@@ -81,11 +88,17 @@ The authenticator configuration examples in the rest of this document will use "
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.initialInternalClientPassword`|Initial [Password Provider](../../operations/password-provider.md) for the default internal system user, used for internal process communication. If no password is specified, the default internal system user will not be created. If the default internal system user already exists, setting this property will not affect its password.|null|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.enableCacheNotifications`|If true, the Coordinator will notify Druid processes whenever a configuration change to this Authenticator occurs, allowing them to immediately update their state without waiting for polling.|true|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.cacheNotificationTimeout`|The timeout in milliseconds for the cache notifications.|5000|No|
-|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialIterations`|Number of iterations to use for password hashing.|10000|No|
+|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialIterations`|Number of iterations to use for password hashing. See [Credential iterations and query times](#credential-iterations-and-query-times)|10000|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialsValidator.type`|The type of credentials store (metadata) to validate requests credentials.|metadata|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.skipOnFailure`|If true and the request credential doesn't exists or isn't fully configured in the credentials store, the request will proceed to next Authenticator in the chain.|false|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.authorizerName`|Authorizer that requests should be directed to|N/A|Yes|
+##### Credential iterations and query times
Review comment:
@suneet-s I have updated this to reflect that the impact is on all API calls, but in the first sentence mention "including queries"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s merged pull request #10532: Document correlation between credential iterations and query latency
Posted by GitBox <gi...@apache.org>.
suneet-s merged pull request #10532:
URL: https://github.com/apache/druid/pull/10532
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #10532: Document correlation between credential iterations and query latency
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #10532:
URL: https://github.com/apache/druid/pull/10532#discussion_r513483867
##########
File path: docs/development/extensions-core/druid-basic-security.md
##########
@@ -81,11 +88,17 @@ The authenticator configuration examples in the rest of this document will use "
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.initialInternalClientPassword`|Initial [Password Provider](../../operations/password-provider.md) for the default internal system user, used for internal process communication. If no password is specified, the default internal system user will not be created. If the default internal system user already exists, setting this property will not affect its password.|null|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.enableCacheNotifications`|If true, the Coordinator will notify Druid processes whenever a configuration change to this Authenticator occurs, allowing them to immediately update their state without waiting for polling.|true|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.cacheNotificationTimeout`|The timeout in milliseconds for the cache notifications.|5000|No|
-|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialIterations`|Number of iterations to use for password hashing.|10000|No|
+|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialIterations`|Number of iterations to use for password hashing. See [Credential iterations and query times](#credential-iterations-and-query-times)|10000|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialsValidator.type`|The type of credentials store (metadata) to validate requests credentials.|metadata|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.skipOnFailure`|If true and the request credential doesn't exists or isn't fully configured in the credentials store, the request will proceed to next Authenticator in the chain.|false|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.authorizerName`|Authorizer that requests should be directed to|N/A|Yes|
+##### Credential iterations and query times
Review comment:
The iterations apply to all API calls, this title makes me think it only impacts query times
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] techdocsmith commented on pull request #10532: Document correlation between credential iterations and query latency
Posted by GitBox <gi...@apache.org>.
techdocsmith commented on pull request #10532:
URL: https://github.com/apache/druid/pull/10532#issuecomment-716904050
@suneet-s , @2bethere PTAL.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] nishantmonu51 commented on a change in pull request #10532: Document correlation between credential iterations and query latency
Posted by GitBox <gi...@apache.org>.
nishantmonu51 commented on a change in pull request #10532:
URL: https://github.com/apache/druid/pull/10532#discussion_r512930964
##########
File path: docs/development/extensions-core/druid-basic-security.md
##########
@@ -62,16 +69,16 @@ druid.auth.authenticator.MyBasicMetadataAuthenticator.authorizerName=MyBasicMeta
```
To use the Basic authenticator, add an authenticator with type `basic` to the authenticatorChain.
-The authenticator needs to also define a credentialsValidator with type 'metadata' or 'ldap'.
-If credentialsValidator is not specified, type 'metadata' will be used as default.
+The authenticator must define a credentials validator `credentialsValidator` with type 'metadata' or 'ldap'.
+Otherwise the credentials validator defaults to `metadata` if you do not specify another type.
Review comment:
above 2 lines seems to be contradictory, first one says you must define it and 2nd mentions a default value.
probably, reword as - "The authenticator uses metadata credentials validator by default which can be configured to `metadata`." Or similar wordings.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #10532: Document correlation between credential iterations and query latency
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #10532:
URL: https://github.com/apache/druid/pull/10532#discussion_r513483867
##########
File path: docs/development/extensions-core/druid-basic-security.md
##########
@@ -81,11 +88,17 @@ The authenticator configuration examples in the rest of this document will use "
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.initialInternalClientPassword`|Initial [Password Provider](../../operations/password-provider.md) for the default internal system user, used for internal process communication. If no password is specified, the default internal system user will not be created. If the default internal system user already exists, setting this property will not affect its password.|null|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.enableCacheNotifications`|If true, the Coordinator will notify Druid processes whenever a configuration change to this Authenticator occurs, allowing them to immediately update their state without waiting for polling.|true|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.cacheNotificationTimeout`|The timeout in milliseconds for the cache notifications.|5000|No|
-|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialIterations`|Number of iterations to use for password hashing.|10000|No|
+|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialIterations`|Number of iterations to use for password hashing. See [Credential iterations and query times](#credential-iterations-and-query-times)|10000|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.credentialsValidator.type`|The type of credentials store (metadata) to validate requests credentials.|metadata|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.skipOnFailure`|If true and the request credential doesn't exists or isn't fully configured in the credentials store, the request will proceed to next Authenticator in the chain.|false|No|
|`druid.auth.authenticator.MyBasicMetadataAuthenticator.authorizerName`|Authorizer that requests should be directed to|N/A|Yes|
+##### Credential iterations and query times
Review comment:
The iterations impact all API calls, this title makes me think it only impacts query response time. Similar comment in the paragraph below. I don't have a better suggestion at this point
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] techdocsmith commented on a change in pull request #10532: Document correlation between credential iterations and query latency
Posted by GitBox <gi...@apache.org>.
techdocsmith commented on a change in pull request #10532:
URL: https://github.com/apache/druid/pull/10532#discussion_r513714301
##########
File path: docs/development/extensions-core/druid-basic-security.md
##########
@@ -62,16 +69,16 @@ druid.auth.authenticator.MyBasicMetadataAuthenticator.authorizerName=MyBasicMeta
```
To use the Basic authenticator, add an authenticator with type `basic` to the authenticatorChain.
-The authenticator needs to also define a credentialsValidator with type 'metadata' or 'ldap'.
-If credentialsValidator is not specified, type 'metadata' will be used as default.
+The authenticator must define a credentials validator `credentialsValidator` with type 'metadata' or 'ldap'.
+Otherwise the credentials validator defaults to `metadata` if you do not specify another type.
Review comment:
@nishantmonu51 , @suneet-s updated this to reflect that `metadata` is the default. And you only need to define one for `LDAP`. PTAL. Not sure if I've 100% got the nuance here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org