[KYLIN-5229] PropertiesDelegate introduce lock to KylinConfig

• Type: Sub-task
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 5.0-alpha
• Component/s: Metadata
• Assignee: mukvin
• Reporter: Xiaoxiang Yu
• Created: 17/Aug/22 05:17
• Updated: 15/Sep/22 01:35
• Resolved: 24/Aug/22 06:30

Description

Background

Kyligence 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.

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.

Root Cause

 

//// KylinConfigBase

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);
}


//// PropertiesDelegate

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



private synchronized Properties getAllProperties() { // Step 4, synchronized     
    Properties propertiesView = new Properties();
    if (this.configLoader != null) {
        propertiesView.putAll(this.configLoader.getProperties()); // Step 5
    }
    propertiesView.putAll(this.properties);
    return propertiesView;
} 


//// Hashtable

public synchronized void putAll(Map<? extends K, ? extends V> t) {
    for (Map.Entry<? extends K, ? extends V> e : t.entrySet())
        put(e.getKey(), e.getValue()); // Step 7
}


public synchronized V put(K key, V value) { // Step 8
    // Make sure the value is not null
    if (value == null) {
        throw new NullPointerException();
    }

    // Makes sure the key is not already in the hashtable.
    Entry<?,?> tab[] = table;
    int hash = key.hashCode();
    int index = (hash & 0x7FFFFFFF) % tab.length;
    @SuppressWarnings("unchecked")
    Entry<K,V> entry = (Entry<K,V>)tab[index];
    for(; entry != null ; entry = entry.next) {
        if ((entry.hash == hash) && entry.key.equals(key)) {
            V old = entry.value;
            entry.value = value;
            return old;
        }
    }

    addEntry(hash, key, value, index);
    return null;
}

 

Screenshots

Java Stack of Query Thread: