[
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
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.
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}
//// 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;
}
{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}
//// 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;
}
{code}
h2. Screenshots
Java Stack of Query Thread:
!image-2022-08-17-13-17-40-751.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
> 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.
> 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}
> //// 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;
> }
> {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)
