[ 
https://issues.apache.org/jira/browse/KYLIN-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Xiaoxiang Yu updated KYLIN-5229:
--------------------------------
    Description: 
h2. Background

Kylin 5.0 dev team plans to add new feature which supports reading config entry 
from external Config Server(such as Nacos) other than local kylin.properties. 
So we add {{PropertiesDelegate}} and {{IExternalConfigLoader}} to support this 
request.
h2. Issue

{{{}PropertiesDelegate }}introduces performance downgrade because it add lock 
to \{{{}KylinConfigBase#getOptional{}}}, impact query concurrency badly, from 
Yanghong Team's load testing result, query concurrency change from 60 to 3.
h2. Root Cause

 
{code:java}
 protected String getOptional(String prop, String dft) {
    final String property = System.getProperty(prop);
    return property != null ? getSubstitutor().replace(property)
            : getSubstitutor().replace(properties.getProperty(prop, dft)); // 
Step 1
}
 
protected StrSubstitutor getSubstitutor() {
    // overrides > env > properties
    final Map<String, Object> all = Maps.newHashMap();
    all.putAll((Map) properties); // Step 2
    all.putAll(STATIC_SYSTEM_ENV);
    all.putAll(overrides);
    return new StrSubstitutor(all);
}


@Override
public Set<Map.Entry<Object, Object>> entrySet() {
    return getAllProperties().entrySet(); // Step 3
}



private synchronized Properties getAllProperties() { // Step4, synchronized     
    Properties propertiesView = new Properties();
    if (this.configLoader != null) {
        propertiesView.putAll(this.configLoader.getProperties());
    }
    propertiesView.putAll(this.properties);
    return propertiesView;
} {code}
 
h2. Screenshots

Java Stack of Query Thread:

  !image-2022-08-17-13-17-40-751.png!

  was:
h2. Background

Kylin 5.0 dev team plans to add new feature which supports reading config entry 
from external Config Server(such as Nacos) other than local kylin.properties. 
So we add {{PropertiesDelegate}} and {{IExternalConfigLoader}} to support this 
request.
h2. Issue

{{PropertiesDelegate }}introduces performance downgrade because it add lock to 
{{{}KylinConfigBase#getOptional{}}}, impact query concurrency badly, from 
Yanghong Team's load testing result, query concurrency change from 60 to 3.
h2. Root Cause

 
{code:java}
 protected String getOptional(String prop, String dft) {
    final String property = System.getProperty(prop);
    return property != null ? getSubstitutor().replace(property)
            : getSubstitutor().replace(properties.getProperty(prop, dft)); // 
Step 1
}
 
protected StrSubstitutor getSubstitutor() {
    // overrides > env > properties
    final Map<String, Object> all = Maps.newHashMap();
    all.putAll((Map) properties); // Step 2
    all.putAll(STATIC_SYSTEM_ENV);
    all.putAll(overrides);
    return new StrSubstitutor(all);
}


@Override
public Set<Map.Entry<Object, Object>> entrySet() {
    return getAllProperties().entrySet(); // Step 3
}



private synchronized Properties getAllProperties() { // Step4, synchronized     
    Properties propertiesView = new Properties();
    if (this.configLoader != null) {
        propertiesView.putAll(this.configLoader.getProperties());
    }
    propertiesView.putAll(this.properties);
    return propertiesView;
} {code}
 
h2. Screenshots

Java Stack of Query Thread:

!image-2022-08-17-13-16-25-721.png!


> PropertiesDelegate introduce lock to KylinConfig
> ------------------------------------------------
>
>                 Key: KYLIN-5229
>                 URL: https://issues.apache.org/jira/browse/KYLIN-5229
>             Project: Kylin
>          Issue Type: Sub-task
>          Components: Metadata
>            Reporter: Xiaoxiang Yu
>            Priority: Major
>             Fix For: 5.0-alpha
>
>         Attachments: image-2022-08-17-13-17-40-751.png
>
>
> h2. Background
> Kylin 5.0 dev team plans to add new feature which supports reading config 
> entry from external Config Server(such as Nacos) other than local 
> kylin.properties. So we add {{PropertiesDelegate}} and 
> {{IExternalConfigLoader}} to support this request.
> h2. Issue
> {{{}PropertiesDelegate }}introduces performance downgrade because it add lock 
> to \{{{}KylinConfigBase#getOptional{}}}, impact query concurrency badly, from 
> Yanghong Team's load testing result, query concurrency change from 60 to 3.
> h2. Root Cause
>  
> {code:java}
>  protected String getOptional(String prop, String dft) {
>     final String property = System.getProperty(prop);
>     return property != null ? getSubstitutor().replace(property)
>             : getSubstitutor().replace(properties.getProperty(prop, dft)); // 
> Step 1
> }
>  
> protected StrSubstitutor getSubstitutor() {
>     // overrides > env > properties
>     final Map<String, Object> all = Maps.newHashMap();
>     all.putAll((Map) properties); // Step 2
>     all.putAll(STATIC_SYSTEM_ENV);
>     all.putAll(overrides);
>     return new StrSubstitutor(all);
> }
> @Override
> public Set<Map.Entry<Object, Object>> entrySet() {
>     return getAllProperties().entrySet(); // Step 3
> }
> private synchronized Properties getAllProperties() { // Step4, synchronized   
>   
>     Properties propertiesView = new Properties();
>     if (this.configLoader != null) {
>         propertiesView.putAll(this.configLoader.getProperties());
>     }
>     propertiesView.putAll(this.properties);
>     return propertiesView;
> } {code}
>  
> h2. Screenshots
> Java Stack of Query Thread:
>   !image-2022-08-17-13-17-40-751.png!



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to