Command line interface to manage distributed properties

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Command line interface to manage distributed properties

Taras Ledkov
Hi,

Motivation: we have to manage SQL distributed property by command line
and introduce common approach to manage distributed properties.
Issue: IGNITE-13186 (see [1])

My proposal is:

Property classes & DistributedConfigurationProcessor changes (see PR [2]):
- introduce PublicProperty interface and implements it at the
PublicSimpleProperty;
- SimpleDistributedPublicProperty. If we create the instance of this
class the property can be managed by command line.

Command line interface:
--property list - prints list of the available properties with
description, e.g.:
         sql.disabledFunctions : Disabled SQL functions
         sql.defaultQueryTimeout : Default query timeout
--property get --name <prop_name> - prints the property value
--property set --name <prop_name> --val <prop_value> - change the
property value.

Possible we have to add the command:
--property reset --name <prop_name> - reset property to default value.

Please your comments.
Please pay your attention to concept & design of the publishing a
property by 'PublicProperty' and set of the new commands.

[1]. https://issues.apache.org/jira/browse/IGNITE-13186
[2]. https://github.com/apache/ignite/pull/8208

--
Taras Ledkov
Mail-To: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Command line interface to manage distributed properties

Nikolay Izhikov-2
Hello, Taras.

It a shame we don’t have a well-written guide for the development of the Ignite management interfaces at the moment.
For now, we have dozen of some management APIs - java, JMX, SQL, control.sh, visorcmd.sh, REST

I think we should support 3 manage interfaces for each new command:

* CMD
* JMX
* SQL

You can take as an example implementation of the `KILL` command [1]

> If we create the instance of this class the property can be managed by command line.

Why do we want to restrict property management somehow?

This operation should be done by the administrator who knows what he or she does.
I think we should provide a way to change any property value without any restriction for admin.
So our users don’t have to wait «one more release» with only change ‘implements SimpleDistributedPublicProperty’ for some property.

[1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=145724615

> 2 сент. 2020 г., в 23:11, Taras Ledkov <[hidden email]> написал(а):
>
> Hi,
>
> Motivation: we have to manage SQL distributed property by command line and introduce common approach to manage distributed properties.
> Issue: IGNITE-13186 (see [1])
>
> My proposal is:
>
> Property classes & DistributedConfigurationProcessor changes (see PR [2]):
> - introduce PublicProperty interface and implements it at the PublicSimpleProperty;
> - SimpleDistributedPublicProperty. If we create the instance of this class the property can be managed by command line.
>
> Command line interface:
> --property list - prints list of the available properties with description, e.g.:
>         sql.disabledFunctions : Disabled SQL functions
>         sql.defaultQueryTimeout : Default query timeout
> --property get --name <prop_name> - prints the property value
> --property set --name <prop_name> --val <prop_value> - change the property value.
>
> Possible we have to add the command:
> --property reset --name <prop_name> - reset property to default value.
>
> Please your comments.
> Please pay your attention to concept & design of the publishing a property by 'PublicProperty' and set of the new commands.
>
> [1]. https://issues.apache.org/jira/browse/IGNITE-13186
> [2]. https://github.com/apache/ignite/pull/8208
>
> --
> Taras Ledkov
> Mail-To: [hidden email]
>

Reply | Threaded
Open this post in threaded view
|

Re: Command line interface to manage distributed properties

Taras Ledkov
Hi,

 >  Why do we want to restrict property management somehow?
I guess some properties (may be future properties) shouldn't be
published through generic cmd line interface.
May be its require separate more complex cmd line commands, some
properties may have dependencies and require complex management not only
set/get.
In this case we can use distributed property without publish one via
simpel cmd line interface.

On 03.09.2020 10:05, Nikolay Izhikov wrote:

> Hello, Taras.
>
> It a shame we don’t have a well-written guide for the development of the Ignite management interfaces at the moment.
> For now, we have dozen of some management APIs - java, JMX, SQL, control.sh, visorcmd.sh, REST
>
> I think we should support 3 manage interfaces for each new command:
>
> * CMD
> * JMX
> * SQL
>
> You can take as an example implementation of the `KILL` command [1]
>
>> If we create the instance of this class the property can be managed by command line.
> Why do we want to restrict property management somehow?
>
> This operation should be done by the administrator who knows what he or she does.
> I think we should provide a way to change any property value without any restriction for admin.
> So our users don’t have to wait «one more release» with only change ‘implements SimpleDistributedPublicProperty’ for some property.
>
> [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=145724615
>
>> 2 сент. 2020 г., в 23:11, Taras Ledkov <[hidden email]> написал(а):
>>
>> Hi,
>>
>> Motivation: we have to manage SQL distributed property by command line and introduce common approach to manage distributed properties.
>> Issue: IGNITE-13186 (see [1])
>>
>> My proposal is:
>>
>> Property classes & DistributedConfigurationProcessor changes (see PR [2]):
>> - introduce PublicProperty interface and implements it at the PublicSimpleProperty;
>> - SimpleDistributedPublicProperty. If we create the instance of this class the property can be managed by command line.
>>
>> Command line interface:
>> --property list - prints list of the available properties with description, e.g.:
>>          sql.disabledFunctions : Disabled SQL functions
>>          sql.defaultQueryTimeout : Default query timeout
>> --property get --name <prop_name> - prints the property value
>> --property set --name <prop_name> --val <prop_value> - change the property value.
>>
>> Possible we have to add the command:
>> --property reset --name <prop_name> - reset property to default value.
>>
>> Please your comments.
>> Please pay your attention to concept & design of the publishing a property by 'PublicProperty' and set of the new commands.
>>
>> [1]. https://issues.apache.org/jira/browse/IGNITE-13186
>> [2]. https://github.com/apache/ignite/pull/8208
>>
>> --
>> Taras Ledkov
>> Mail-To: [hidden email]
>>
--
Taras Ledkov
Mail-To: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Command line interface to manage distributed properties

Nikolay Izhikov-2
Hello, Taras.

> I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.

With marker interface user have to wait for a new release to fix not published property.
New release is a very long way for fixing one tiny configuration value.

Also, we shouldn’t hide anything from the administrator.

I’m sure that hiding any internals from our users is always a bad idea and hides some issue in the codebase.
Let’s do it in Apache Way? :) - «Not restriction but common sense»

We can have some kind of `IgniteSystemProperty` with default read-only list and description to it -
«User, you edit this properties fully on your own. We can’t predict results of such kind of edits»
So user can fix this list manually.

WDYT?

> 3 сент. 2020 г., в 10:21, Taras Ledkov <[hidden email]> написал(а):
>
> Hi,
>
> >  Why do we want to restrict property management somehow?
> I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
> May be its require separate more complex cmd line commands, some properties may have dependencies and require complex management not only set/get.
> In this case we can use distributed property without publish one via simpel cmd line interface.
>
> On 03.09.2020 10:05, Nikolay Izhikov wrote:
>> Hello, Taras.
>>
>> It a shame we don’t have a well-written guide for the development of the Ignite management interfaces at the moment.
>> For now, we have dozen of some management APIs - java, JMX, SQL, control.sh, visorcmd.sh, REST
>>
>> I think we should support 3 manage interfaces for each new command:
>>
>> * CMD
>> * JMX
>> * SQL
>>
>> You can take as an example implementation of the `KILL` command [1]
>>
>>> If we create the instance of this class the property can be managed by command line.
>> Why do we want to restrict property management somehow?
>>
>> This operation should be done by the administrator who knows what he or she does.
>> I think we should provide a way to change any property value without any restriction for admin.
>> So our users don’t have to wait «one more release» with only change ‘implements SimpleDistributedPublicProperty’ for some property.
>>
>> [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=145724615
>>
>>> 2 сент. 2020 г., в 23:11, Taras Ledkov <[hidden email]> написал(а):
>>>
>>> Hi,
>>>
>>> Motivation: we have to manage SQL distributed property by command line and introduce common approach to manage distributed properties.
>>> Issue: IGNITE-13186 (see [1])
>>>
>>> My proposal is:
>>>
>>> Property classes & DistributedConfigurationProcessor changes (see PR [2]):
>>> - introduce PublicProperty interface and implements it at the PublicSimpleProperty;
>>> - SimpleDistributedPublicProperty. If we create the instance of this class the property can be managed by command line.
>>>
>>> Command line interface:
>>> --property list - prints list of the available properties with description, e.g.:
>>>         sql.disabledFunctions : Disabled SQL functions
>>>         sql.defaultQueryTimeout : Default query timeout
>>> --property get --name <prop_name> - prints the property value
>>> --property set --name <prop_name> --val <prop_value> - change the property value.
>>>
>>> Possible we have to add the command:
>>> --property reset --name <prop_name> - reset property to default value.
>>>
>>> Please your comments.
>>> Please pay your attention to concept & design of the publishing a property by 'PublicProperty' and set of the new commands.
>>>
>>> [1]. https://issues.apache.org/jira/browse/IGNITE-13186
>>> [2]. https://github.com/apache/ignite/pull/8208
>>>
>>> --
>>> Taras Ledkov
>>> Mail-To: [hidden email]
>>>
> --
> Taras Ledkov
> Mail-To: [hidden email]
>

Reply | Threaded
Open this post in threaded view
|

Re: Command line interface to manage distributed properties

Nikolay Izhikov-2
Hello, Taras.

One more thing:

> --property list - prints list of the available properties with description, e.g.:

We have a convenient API to show Ignite internal objects - System Views [1]

Any system view available via SQL and JMX.
It seems we should have METASTORAGE view instead of this option.

P.S. Should we add some CMD interface for system views?

[1] https://apacheignite.readme.io/docs/system-views

> 3 сент. 2020 г., в 10:37, Nikolay Izhikov <[hidden email]> написал(а):
>
> Hello, Taras.
>
>> I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
>
> With marker interface user have to wait for a new release to fix not published property.
> New release is a very long way for fixing one tiny configuration value.
>
> Also, we shouldn’t hide anything from the administrator.
>
> I’m sure that hiding any internals from our users is always a bad idea and hides some issue in the codebase.
> Let’s do it in Apache Way? :) - «Not restriction but common sense»
>
> We can have some kind of `IgniteSystemProperty` with default read-only list and description to it -
> «User, you edit this properties fully on your own. We can’t predict results of such kind of edits»
> So user can fix this list manually.
>
> WDYT?
>
>> 3 сент. 2020 г., в 10:21, Taras Ledkov <[hidden email]> написал(а):
>>
>> Hi,
>>
>>> Why do we want to restrict property management somehow?
>> I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
>> May be its require separate more complex cmd line commands, some properties may have dependencies and require complex management not only set/get.
>> In this case we can use distributed property without publish one via simpel cmd line interface.
>>
>> On 03.09.2020 10:05, Nikolay Izhikov wrote:
>>> Hello, Taras.
>>>
>>> It a shame we don’t have a well-written guide for the development of the Ignite management interfaces at the moment.
>>> For now, we have dozen of some management APIs - java, JMX, SQL, control.sh, visorcmd.sh, REST
>>>
>>> I think we should support 3 manage interfaces for each new command:
>>>
>>> * CMD
>>> * JMX
>>> * SQL
>>>
>>> You can take as an example implementation of the `KILL` command [1]
>>>
>>>> If we create the instance of this class the property can be managed by command line.
>>> Why do we want to restrict property management somehow?
>>>
>>> This operation should be done by the administrator who knows what he or she does.
>>> I think we should provide a way to change any property value without any restriction for admin.
>>> So our users don’t have to wait «one more release» with only change ‘implements SimpleDistributedPublicProperty’ for some property.
>>>
>>> [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=145724615
>>>
>>>> 2 сент. 2020 г., в 23:11, Taras Ledkov <[hidden email]> написал(а):
>>>>
>>>> Hi,
>>>>
>>>> Motivation: we have to manage SQL distributed property by command line and introduce common approach to manage distributed properties.
>>>> Issue: IGNITE-13186 (see [1])
>>>>
>>>> My proposal is:
>>>>
>>>> Property classes & DistributedConfigurationProcessor changes (see PR [2]):
>>>> - introduce PublicProperty interface and implements it at the PublicSimpleProperty;
>>>> - SimpleDistributedPublicProperty. If we create the instance of this class the property can be managed by command line.
>>>>
>>>> Command line interface:
>>>> --property list - prints list of the available properties with description, e.g.:
>>>>        sql.disabledFunctions : Disabled SQL functions
>>>>        sql.defaultQueryTimeout : Default query timeout
>>>> --property get --name <prop_name> - prints the property value
>>>> --property set --name <prop_name> --val <prop_value> - change the property value.
>>>>
>>>> Possible we have to add the command:
>>>> --property reset --name <prop_name> - reset property to default value.
>>>>
>>>> Please your comments.
>>>> Please pay your attention to concept & design of the publishing a property by 'PublicProperty' and set of the new commands.
>>>>
>>>> [1]. https://issues.apache.org/jira/browse/IGNITE-13186
>>>> [2]. https://github.com/apache/ignite/pull/8208
>>>>
>>>> --
>>>> Taras Ledkov
>>>> Mail-To: [hidden email]
>>>>
>> --
>> Taras Ledkov
>> Mail-To: [hidden email]
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: Command line interface to manage distributed properties

Anton Kalashnikov
Hi everyone,

I think I agree with Nikolay that we should make available all our property not only some of them. Also maybe it makes sense to split this task into two tasks in the following way: Publish all distributed property through public interfaces(control.sh, jmx, etc.), Giving possibility to add permission to some properties.

In my opinion, we need the following changes(it is just a high overview of my ideas):

1) Publish distributed property
- DistributedConfigurationProcessor new methods: propertyList(): List<DistributedProperty>, get(propertyName): DistributedProperty
- DistributedProperty new methods: stringView(), propagateFromString(valueAsString) - it's discussable, I still don't sure it is great place for such methods. Perhaps we should have a some converter inside of control.sh or whatever.

Usage:
List<DistributedProperty> allClusterProperties = distributedConfigurationProcessor.list();
allClusterProperties.forEach( prop -> System.out.printl(prop.stringView()) );

DistributedProperty baselineAutoAdjustEnabled = distributedConfigurationProcessor.get("baselineAutoAdjustEnabled");
baselineAutoAdjustEnabled.propagateFromString("true");//baselineAutoAdjustEnabled.propagate(true)

Open questions:
- How to update complex objects(any object which is not primitive)? 
- Does it ok to convert the object to string inside the DistributedProperty?

2) Permission for distributed properties
- New class for permission - unfortunately, it's broking all hierarchy(DistributedLongProperty, DistributedBooleanProperty etc.) but maybe it is not a big problem
class PermissibleDistributedProperty<T> extends SimpleDistributedProperty<InnerPermissionWrapper<T>> {
   PermittedDistributedProperty(key, realValue, readPermission, writePermission) {
      super(key, new InnerPermissionWrapper(realValue, readPermission, writePermission);
    }
}

- I don't know a lot about ignite security so I don't sure where we should check the permission in that case - it can be a new special processor or just inside a job

One more idea - instead of creating the PermittedDistributedProperty we can store some mapping <property, readPermission, writePermission> separately(it can be static mapping or it can be stored in some new DistributedProperty). But in this case, it is possible to lost permission after property renaming.

-- 
Best regards,
Anton Kalashnikov



05.09.2020, 10:09, "Nikolay Izhikov" <[hidden email]>:

> Hello, Taras.
>
> One more thing:
>
>>  --property list - prints list of the available properties with description, e.g.:
>
> We have a convenient API to show Ignite internal objects - System Views [1]
>
> Any system view available via SQL and JMX.
> It seems we should have METASTORAGE view instead of this option.
>
> P.S. Should we add some CMD interface for system views?
>
> [1] https://apacheignite.readme.io/docs/system-views
>
>>  3 сент. 2020 г., в 10:37, Nikolay Izhikov <[hidden email]> написал(а):
>>
>>  Hello, Taras.
>>
>>>  I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
>>
>>  With marker interface user have to wait for a new release to fix not published property.
>>  New release is a very long way for fixing one tiny configuration value.
>>
>>  Also, we shouldn’t hide anything from the administrator.
>>
>>  I’m sure that hiding any internals from our users is always a bad idea and hides some issue in the codebase.
>>  Let’s do it in Apache Way? :) - «Not restriction but common sense»
>>
>>  We can have some kind of `IgniteSystemProperty` with default read-only list and description to it -
>>  «User, you edit this properties fully on your own. We can’t predict results of such kind of edits»
>>  So user can fix this list manually.
>>
>>  WDYT?
>>
>>>  3 сент. 2020 г., в 10:21, Taras Ledkov <[hidden email]> написал(а):
>>>
>>>  Hi,
>>>
>>>>  Why do we want to restrict property management somehow?
>>>  I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
>>>  May be its require separate more complex cmd line commands, some properties may have dependencies and require complex management not only set/get.
>>>  In this case we can use distributed property without publish one via simpel cmd line interface.
>>>
>>>  On 03.09.2020 10:05, Nikolay Izhikov wrote:
>>>>  Hello, Taras.
>>>>
>>>>  It a shame we don’t have a well-written guide for the development of the Ignite management interfaces at the moment.
>>>>  For now, we have dozen of some management APIs - java, JMX, SQL, control.sh, visorcmd.sh, REST
>>>>
>>>>  I think we should support 3 manage interfaces for each new command:
>>>>
>>>>  * CMD
>>>>  * JMX
>>>>  * SQL
>>>>
>>>>  You can take as an example implementation of the `KILL` command [1]
>>>>
>>>>>  If we create the instance of this class the property can be managed by command line.
>>>>  Why do we want to restrict property management somehow?
>>>>
>>>>  This operation should be done by the administrator who knows what he or she does.
>>>>  I think we should provide a way to change any property value without any restriction for admin.
>>>>  So our users don’t have to wait «one more release» with only change ‘implements SimpleDistributedPublicProperty’ for some property.
>>>>
>>>>  [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=145724615
>>>>
>>>>>  2 сент. 2020 г., в 23:11, Taras Ledkov <[hidden email]> написал(а):
>>>>>
>>>>>  Hi,
>>>>>
>>>>>  Motivation: we have to manage SQL distributed property by command line and introduce common approach to manage distributed properties.
>>>>>  Issue: IGNITE-13186 (see [1])
>>>>>
>>>>>  My proposal is:
>>>>>
>>>>>  Property classes & DistributedConfigurationProcessor changes (see PR [2]):
>>>>>  - introduce PublicProperty interface and implements it at the PublicSimpleProperty;
>>>>>  - SimpleDistributedPublicProperty. If we create the instance of this class the property can be managed by command line.
>>>>>
>>>>>  Command line interface:
>>>>>  --property list - prints list of the available properties with description, e.g.:
>>>>>         sql.disabledFunctions : Disabled SQL functions
>>>>>         sql.defaultQueryTimeout : Default query timeout
>>>>>  --property get --name <prop_name> - prints the property value
>>>>>  --property set --name <prop_name> --val <prop_value> - change the property value.
>>>>>
>>>>>  Possible we have to add the command:
>>>>>  --property reset --name <prop_name> - reset property to default value.
>>>>>
>>>>>  Please your comments.
>>>>>  Please pay your attention to concept & design of the publishing a property by 'PublicProperty' and set of the new commands.
>>>>>
>>>>>  [1]. https://issues.apache.org/jira/browse/IGNITE-13186
>>>>>  [2]. https://github.com/apache/ignite/pull/8208
>>>>>
>>>>>  --
>>>>>  Taras Ledkov
>>>>>  Mail-To: [hidden email]
>>>  --
>>>  Taras Ledkov
>>>  Mail-To: [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: Command line interface to manage distributed properties

Nikolay Izhikov-2
Hello, Anton.

> 1) Publish distributed property

I propose to use SystemView API to accomplish this.
Actually, there is PR for it, already [1]

Distributed property will be available via SQL - «SELECT * FROM SYS.DISTRIBUTED_METASTORAGE»
Or via JMX in the corresponding bean.

> 2) Permission for distributed properties

Do we really need separate permission for each property?
Would it be enough to have one permission «DISTRIBUTED_PROPERTY_WRITE» or similar?
WIth it we allow to the user SET operation for any property.

[1] https://github.com/apache/ignite/pull/8225/files

> 8 сент. 2020 г., в 16:53, Anton Kalashnikov <[hidden email]> написал(а):
>
> Hi everyone,
>
> I think I agree with Nikolay that we should make available all our property not only some of them. Also maybe it makes sense to split this task into two tasks in the following way: Publish all distributed property through public interfaces(control.sh, jmx, etc.), Giving possibility to add permission to some properties.
>
> In my opinion, we need the following changes(it is just a high overview of my ideas):
>
> 1) Publish distributed property
> - DistributedConfigurationProcessor new methods: propertyList(): List<DistributedProperty>, get(propertyName): DistributedProperty
> - DistributedProperty new methods: stringView(), propagateFromString(valueAsString) - it's discussable, I still don't sure it is great place for such methods. Perhaps we should have a some converter inside of control.sh or whatever.
>
> Usage:
> List<DistributedProperty> allClusterProperties = distributedConfigurationProcessor.list();
> allClusterProperties.forEach( prop -> System.out.printl(prop.stringView()) );
>
> DistributedProperty baselineAutoAdjustEnabled = distributedConfigurationProcessor.get("baselineAutoAdjustEnabled");
> baselineAutoAdjustEnabled.propagateFromString("true");//baselineAutoAdjustEnabled.propagate(true)
>
> Open questions:
> - How to update complex objects(any object which is not primitive)?
> - Does it ok to convert the object to string inside the DistributedProperty?
>
> 2) Permission for distributed properties
> - New class for permission - unfortunately, it's broking all hierarchy(DistributedLongProperty, DistributedBooleanProperty etc.) but maybe it is not a big problem
> class PermissibleDistributedProperty<T> extends SimpleDistributedProperty<InnerPermissionWrapper<T>> {
>    PermittedDistributedProperty(key, realValue, readPermission, writePermission) {
>       super(key, new InnerPermissionWrapper(realValue, readPermission, writePermission);
>     }
> }
>
> - I don't know a lot about ignite security so I don't sure where we should check the permission in that case - it can be a new special processor or just inside a job
>
> One more idea - instead of creating the PermittedDistributedProperty we can store some mapping <property, readPermission, writePermission> separately(it can be static mapping or it can be stored in some new DistributedProperty). But in this case, it is possible to lost permission after property renaming.
>
> --
> Best regards,
> Anton Kalashnikov
>
>
>
> 05.09.2020, 10:09, "Nikolay Izhikov" <[hidden email]>:
>> Hello, Taras.
>>
>> One more thing:
>>
>>>  --property list - prints list of the available properties with description, e.g.:
>>
>> We have a convenient API to show Ignite internal objects - System Views [1]
>>
>> Any system view available via SQL and JMX.
>> It seems we should have METASTORAGE view instead of this option.
>>
>> P.S. Should we add some CMD interface for system views?
>>
>> [1] https://apacheignite.readme.io/docs/system-views
>>
>>>  3 сент. 2020 г., в 10:37, Nikolay Izhikov <[hidden email]> написал(а):
>>>
>>>  Hello, Taras.
>>>
>>>>  I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
>>>
>>>  With marker interface user have to wait for a new release to fix not published property.
>>>  New release is a very long way for fixing one tiny configuration value.
>>>
>>>  Also, we shouldn’t hide anything from the administrator.
>>>
>>>  I’m sure that hiding any internals from our users is always a bad idea and hides some issue in the codebase.
>>>  Let’s do it in Apache Way? :) - «Not restriction but common sense»
>>>
>>>  We can have some kind of `IgniteSystemProperty` with default read-only list and description to it -
>>>  «User, you edit this properties fully on your own. We can’t predict results of such kind of edits»
>>>  So user can fix this list manually.
>>>
>>>  WDYT?
>>>
>>>>  3 сент. 2020 г., в 10:21, Taras Ledkov <[hidden email]> написал(а):
>>>>
>>>>  Hi,
>>>>
>>>>>  Why do we want to restrict property management somehow?
>>>>  I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
>>>>  May be its require separate more complex cmd line commands, some properties may have dependencies and require complex management not only set/get.
>>>>  In this case we can use distributed property without publish one via simpel cmd line interface.
>>>>
>>>>  On 03.09.2020 10:05, Nikolay Izhikov wrote:
>>>>>  Hello, Taras.
>>>>>
>>>>>  It a shame we don’t have a well-written guide for the development of the Ignite management interfaces at the moment.
>>>>>  For now, we have dozen of some management APIs - java, JMX, SQL, control.sh, visorcmd.sh, REST
>>>>>
>>>>>  I think we should support 3 manage interfaces for each new command:
>>>>>
>>>>>  * CMD
>>>>>  * JMX
>>>>>  * SQL
>>>>>
>>>>>  You can take as an example implementation of the `KILL` command [1]
>>>>>
>>>>>>  If we create the instance of this class the property can be managed by command line.
>>>>>  Why do we want to restrict property management somehow?
>>>>>
>>>>>  This operation should be done by the administrator who knows what he or she does.
>>>>>  I think we should provide a way to change any property value without any restriction for admin.
>>>>>  So our users don’t have to wait «one more release» with only change ‘implements SimpleDistributedPublicProperty’ for some property.
>>>>>
>>>>>  [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=145724615
>>>>>
>>>>>>  2 сент. 2020 г., в 23:11, Taras Ledkov <[hidden email]> написал(а):
>>>>>>
>>>>>>  Hi,
>>>>>>
>>>>>>  Motivation: we have to manage SQL distributed property by command line and introduce common approach to manage distributed properties.
>>>>>>  Issue: IGNITE-13186 (see [1])
>>>>>>
>>>>>>  My proposal is:
>>>>>>
>>>>>>  Property classes & DistributedConfigurationProcessor changes (see PR [2]):
>>>>>>  - introduce PublicProperty interface and implements it at the PublicSimpleProperty;
>>>>>>  - SimpleDistributedPublicProperty. If we create the instance of this class the property can be managed by command line.
>>>>>>
>>>>>>  Command line interface:
>>>>>>  --property list - prints list of the available properties with description, e.g.:
>>>>>>         sql.disabledFunctions : Disabled SQL functions
>>>>>>         sql.defaultQueryTimeout : Default query timeout
>>>>>>  --property get --name <prop_name> - prints the property value
>>>>>>  --property set --name <prop_name> --val <prop_value> - change the property value.
>>>>>>
>>>>>>  Possible we have to add the command:
>>>>>>  --property reset --name <prop_name> - reset property to default value.
>>>>>>
>>>>>>  Please your comments.
>>>>>>  Please pay your attention to concept & design of the publishing a property by 'PublicProperty' and set of the new commands.
>>>>>>
>>>>>>  [1]. https://issues.apache.org/jira/browse/IGNITE-13186
>>>>>>  [2]. https://github.com/apache/ignite/pull/8208
>>>>>>
>>>>>>  --
>>>>>>  Taras Ledkov
>>>>>>  Mail-To: [hidden email]
>>>>  --
>>>>  Taras Ledkov
>>>>  Mail-To: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Command line interface to manage distributed properties

Taras Ledkov
Hi,

Lets finalize our decision.

1. All distributed properties are published. - It's clear.

2. Do we add the description for the property (add method to the
interface DistributedProperty)?
If yes: is the description is persisted or hard-coded?

3. Permission:
- add one permission to modify all properties (the value of any property
may be read without permission)
- separate permission for each property.

I'm OK with both cases. It's obvious that one permission for all
properties is easier to implement.

On 08.09.2020 17:19, Nikolay Izhikov wrote:

> Hello, Anton.
>
>> 1) Publish distributed property
> I propose to use SystemView API to accomplish this.
> Actually, there is PR for it, already [1]
>
> Distributed property will be available via SQL - «SELECT * FROM SYS.DISTRIBUTED_METASTORAGE»
> Or via JMX in the corresponding bean.
>
>> 2) Permission for distributed properties
> Do we really need separate permission for each property?
> Would it be enough to have one permission «DISTRIBUTED_PROPERTY_WRITE» or similar?
> WIth it we allow to the user SET operation for any property.
>
> [1] https://github.com/apache/ignite/pull/8225/files
>
>> 8 сент. 2020 г., в 16:53, Anton Kalashnikov <[hidden email]> написал(а):
>>
>> Hi everyone,
>>
>> I think I agree with Nikolay that we should make available all our property not only some of them. Also maybe it makes sense to split this task into two tasks in the following way: Publish all distributed property through public interfaces(control.sh, jmx, etc.), Giving possibility to add permission to some properties.
>>
>> In my opinion, we need the following changes(it is just a high overview of my ideas):
>>
>> 1) Publish distributed property
>> - DistributedConfigurationProcessor new methods: propertyList(): List<DistributedProperty>, get(propertyName): DistributedProperty
>> - DistributedProperty new methods: stringView(), propagateFromString(valueAsString) - it's discussable, I still don't sure it is great place for such methods. Perhaps we should have a some converter inside of control.sh or whatever.
>>
>> Usage:
>> List<DistributedProperty> allClusterProperties = distributedConfigurationProcessor.list();
>> allClusterProperties.forEach( prop -> System.out.printl(prop.stringView()) );
>>
>> DistributedProperty baselineAutoAdjustEnabled = distributedConfigurationProcessor.get("baselineAutoAdjustEnabled");
>> baselineAutoAdjustEnabled.propagateFromString("true");//baselineAutoAdjustEnabled.propagate(true)
>>
>> Open questions:
>> - How to update complex objects(any object which is not primitive)?
>> - Does it ok to convert the object to string inside the DistributedProperty?
>>
>> 2) Permission for distributed properties
>> - New class for permission - unfortunately, it's broking all hierarchy(DistributedLongProperty, DistributedBooleanProperty etc.) but maybe it is not a big problem
>> class PermissibleDistributedProperty<T> extends SimpleDistributedProperty<InnerPermissionWrapper<T>> {
>>     PermittedDistributedProperty(key, realValue, readPermission, writePermission) {
>>        super(key, new InnerPermissionWrapper(realValue, readPermission, writePermission);
>>      }
>> }
>>
>> - I don't know a lot about ignite security so I don't sure where we should check the permission in that case - it can be a new special processor or just inside a job
>>
>> One more idea - instead of creating the PermittedDistributedProperty we can store some mapping <property, readPermission, writePermission> separately(it can be static mapping or it can be stored in some new DistributedProperty). But in this case, it is possible to lost permission after property renaming.
>>
>> --
>> Best regards,
>> Anton Kalashnikov
>>
>>
>>
>> 05.09.2020, 10:09, "Nikolay Izhikov" <[hidden email]>:
>>> Hello, Taras.
>>>
>>> One more thing:
>>>
>>>>   --property list - prints list of the available properties with description, e.g.:
>>> We have a convenient API to show Ignite internal objects - System Views [1]
>>>
>>> Any system view available via SQL and JMX.
>>> It seems we should have METASTORAGE view instead of this option.
>>>
>>> P.S. Should we add some CMD interface for system views?
>>>
>>> [1] https://apacheignite.readme.io/docs/system-views
>>>
>>>>   3 сент. 2020 г., в 10:37, Nikolay Izhikov <[hidden email]> написал(а):
>>>>
>>>>   Hello, Taras.
>>>>
>>>>>   I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
>>>>   With marker interface user have to wait for a new release to fix not published property.
>>>>   New release is a very long way for fixing one tiny configuration value.
>>>>
>>>>   Also, we shouldn’t hide anything from the administrator.
>>>>
>>>>   I’m sure that hiding any internals from our users is always a bad idea and hides some issue in the codebase.
>>>>   Let’s do it in Apache Way? :) - «Not restriction but common sense»
>>>>
>>>>   We can have some kind of `IgniteSystemProperty` with default read-only list and description to it -
>>>>   «User, you edit this properties fully on your own. We can’t predict results of such kind of edits»
>>>>   So user can fix this list manually.
>>>>
>>>>   WDYT?
>>>>
>>>>>   3 сент. 2020 г., в 10:21, Taras Ledkov <[hidden email]> написал(а):
>>>>>
>>>>>   Hi,
>>>>>
>>>>>>   Why do we want to restrict property management somehow?
>>>>>   I guess some properties (may be future properties) shouldn't be published through generic cmd line interface.
>>>>>   May be its require separate more complex cmd line commands, some properties may have dependencies and require complex management not only set/get.
>>>>>   In this case we can use distributed property without publish one via simpel cmd line interface.
>>>>>
>>>>>   On 03.09.2020 10:05, Nikolay Izhikov wrote:
>>>>>>   Hello, Taras.
>>>>>>
>>>>>>   It a shame we don’t have a well-written guide for the development of the Ignite management interfaces at the moment.
>>>>>>   For now, we have dozen of some management APIs - java, JMX, SQL, control.sh, visorcmd.sh, REST
>>>>>>
>>>>>>   I think we should support 3 manage interfaces for each new command:
>>>>>>
>>>>>>   * CMD
>>>>>>   * JMX
>>>>>>   * SQL
>>>>>>
>>>>>>   You can take as an example implementation of the `KILL` command [1]
>>>>>>
>>>>>>>   If we create the instance of this class the property can be managed by command line.
>>>>>>   Why do we want to restrict property management somehow?
>>>>>>
>>>>>>   This operation should be done by the administrator who knows what he or she does.
>>>>>>   I think we should provide a way to change any property value without any restriction for admin.
>>>>>>   So our users don’t have to wait «one more release» with only change ‘implements SimpleDistributedPublicProperty’ for some property.
>>>>>>
>>>>>>   [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=145724615
>>>>>>
>>>>>>>   2 сент. 2020 г., в 23:11, Taras Ledkov <[hidden email]> написал(а):
>>>>>>>
>>>>>>>   Hi,
>>>>>>>
>>>>>>>   Motivation: we have to manage SQL distributed property by command line and introduce common approach to manage distributed properties.
>>>>>>>   Issue: IGNITE-13186 (see [1])
>>>>>>>
>>>>>>>   My proposal is:
>>>>>>>
>>>>>>>   Property classes & DistributedConfigurationProcessor changes (see PR [2]):
>>>>>>>   - introduce PublicProperty interface and implements it at the PublicSimpleProperty;
>>>>>>>   - SimpleDistributedPublicProperty. If we create the instance of this class the property can be managed by command line.
>>>>>>>
>>>>>>>   Command line interface:
>>>>>>>   --property list - prints list of the available properties with description, e.g.:
>>>>>>>          sql.disabledFunctions : Disabled SQL functions
>>>>>>>          sql.defaultQueryTimeout : Default query timeout
>>>>>>>   --property get --name <prop_name> - prints the property value
>>>>>>>   --property set --name <prop_name> --val <prop_value> - change the property value.
>>>>>>>
>>>>>>>   Possible we have to add the command:
>>>>>>>   --property reset --name <prop_name> - reset property to default value.
>>>>>>>
>>>>>>>   Please your comments.
>>>>>>>   Please pay your attention to concept & design of the publishing a property by 'PublicProperty' and set of the new commands.
>>>>>>>
>>>>>>>   [1]. https://issues.apache.org/jira/browse/IGNITE-13186
>>>>>>>   [2]. https://github.com/apache/ignite/pull/8208
>>>>>>>
>>>>>>>   --
>>>>>>>   Taras Ledkov
>>>>>>>   Mail-To: [hidden email]
>>>>>   --
>>>>>   Taras Ledkov
>>>>>   Mail-To: [hidden email]

--
Taras Ledkov
Mail-To: [hidden email]