Skip to content

Stop flooding the scheduled executor with no-op tasks for change count updates #486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tjwatson merged 5 commits into from
Mar 27, 2026
+225 −79
Merged

Stop flooding the scheduled executor with no-op tasks for change count updates #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4d68c01
Use messages with parameters
tjwatson Mar 11, 2026
a07a307
Use repeating task to update change count service property
tjwatson Mar 12, 2026
51de0a8
Cancel scheduled task for change count update when inactive
tjwatson Mar 24, 2026
e786254
Use an allowable min delay of 100 ms
tjwatson Mar 25, 2026
ec96dd3
Add test for min timeout for change count update
tjwatson Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,7 @@ public void addServiceListener(String serviceFilterString,
ListenerInfo listenerInfo;
synchronized ( listenerMap )
{
logger.log(Level.DEBUG, "serviceFilterString: " + serviceFilterString,
null);
logger.log(Level.DEBUG, "serviceFilterString: {0}", null, serviceFilterString);
listenerInfo = listenerMap.get( serviceFilterString );
if ( listenerInfo == null )
{
Expand Down
4 changes: 2 additions & 2 deletions scr/src/main/java/org/apache/felix/scr/impl/ComponentActorExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ public Thread newThread(Runnable r)
@Override
protected void beforeExecute(Thread t, Runnable r)
{
logger.log(Level.DEBUG, "Running task: " + r, null);
logger.log(Level.DEBUG, "Running task: {0}", null, r);
}

@Override
protected void afterExecute(Runnable r, Throwable t)
{
if (t != null)
{
logger.log(Level.ERROR, "Unexpected problem executing task " + r, t);
logger.log(Level.ERROR, "Unexpected problem executing task {0}", t, r);
}
}
}
139 changes: 104 additions & 35 deletions scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;

Expand Down Expand Up @@ -132,9 +133,12 @@ public class ComponentRegistry

private final ScheduledExecutorService m_componentActor;

private final UpdateChangeCountProperty m_updateChangeCountPropertyTask;

public ComponentRegistry(final ScrConfiguration scrConfiguration, final ScrLogger logger, final ScheduledExecutorService componentActor )
{
m_configuration = scrConfiguration;
m_updateChangeCountPropertyTask = new UpdateChangeCountProperty(m_configuration.serviceChangecountTimeout(), logger, componentActor);
m_logger = logger;
m_componentActor = componentActor;
m_componentHoldersByName = new HashMap<>();
Expand Down Expand Up @@ -704,54 +708,119 @@ public void unregisterRegionConfigurationSupport(

}

private final AtomicLong changeCount = new AtomicLong();

private volatile ServiceRegistration<ServiceComponentRuntime> registration;

public Dictionary<String, Object> getServiceRegistrationProperties()
Dictionary<String, Object> getServiceRegistrationProperties()
{
final Dictionary<String, Object> props = new Hashtable<>();
props.put(PROP_CHANGECOUNT, this.changeCount.get());

return props;
return m_updateChangeCountPropertyTask.getServiceRegistrationProperties();
}

public void setRegistration(final ServiceRegistration<ServiceComponentRuntime> reg)
public void setRegistration(final ServiceRegistration<ServiceComponentRuntime> reg)
{
this.registration = reg;
m_updateChangeCountPropertyTask.setRegistration(reg);
m_updateChangeCountPropertyTask.schedule();
}

public void updateChangeCount()
{
if ( registration != null )
m_updateChangeCountPropertyTask.updateChangeCount();
}

static class UpdateChangeCountProperty implements Runnable {
// TODO 1 seems really low?
private static final long MIN_ALLOWED_DELAY = 1;
private final AtomicLong changeCount = new AtomicLong();
private volatile ServiceRegistration<ServiceComponentRuntime> registration;
private final long maxNumberOfNoChanges;
private final long delay;
private final ScrLogger logger;
private final ScheduledExecutorService executor;

// guarded by this
private int noChangesCount = 0;
// guarded by this
private ScheduledFuture<?> scheduledFuture = null;

UpdateChangeCountProperty(long delay, ScrLogger logger, ScheduledExecutorService executor)
{
final long count = this.changeCount.incrementAndGet();
this.logger = logger;
this.executor = executor;
if (delay < MIN_ALLOWED_DELAY) {
logger.log(Level.INFO,
"The service change count timeout {0} is less than the allowable minimum {1}. Using the allowable minimum instead.", null,
delay, MIN_ALLOWED_DELAY);
delay = MIN_ALLOWED_DELAY;
}
this.delay = delay;
// Calculate the max number of no changes; must be at least 1 to avoid missing events
// The calculation is intended to let at least 10 seconds pass before canceling the scheduledFuture
maxNumberOfNoChanges = Long.max(10000 / delay, 1);
}

try
{
m_componentActor.schedule(new Runnable()
{
void setRegistration(ServiceRegistration<ServiceComponentRuntime> reg)
{
this.registration = reg;
}

@Override
public void run()
{
if ( changeCount.get() == count )
{
try
{
registration.setProperties(getServiceRegistrationProperties());
}
catch ( final IllegalStateException ise)
{
// we ignore this as this might happen on shutdown
}
Dictionary<String, Object> getServiceRegistrationProperties()
{
final Dictionary<String, Object> props = new Hashtable<>();
props.put(PROP_CHANGECOUNT, this.changeCount.get());

return props;
}

public void updateChangeCount() {
this.changeCount.incrementAndGet();
schedule();
}
synchronized void schedule()
{
// reset noChangesCount to ensure task runs at least once more if it exists
noChangesCount = 0;
if (scheduledFuture != null) {
return;
}
scheduledFuture = executor.scheduleWithFixedDelay(this , delay, delay, TimeUnit.MILLISECONDS);
}

@Override
public void run()
{
ServiceRegistration<ServiceComponentRuntime> currentReg = registration;
if (currentReg == null) {
return;
}
try {
Long registeredChangeCount = null;
try {
registeredChangeCount = (Long) currentReg.getReference().getProperty(PROP_CHANGECOUNT);
} catch ( final IllegalStateException ise) {
// we ignore this as this might happen on shutdown
}
if (registeredChangeCount == null || registeredChangeCount.longValue() != changeCount.get()) {
try
{
currentReg.setProperties(getServiceRegistrationProperties());
}
catch ( final IllegalStateException ise)
{
// we ignore this as this might happen on shutdown
}
} else {
synchronized (this) {
noChangesCount++;
if (noChangesCount > maxNumberOfNoChanges) {
// haven't had any changes for max number of tries;
// cancel the scheduled future if it exists.
if (scheduledFuture != null) {
scheduledFuture.cancel(false);
scheduledFuture = null;
}
}
}, m_configuration.serviceChangecountTimeout(), TimeUnit.MILLISECONDS);
}
catch (Exception e) {
m_logger.log(Level.WARN,
"Service changecount Timer for {0} had a problem", e,
}
}
} catch (Exception e) {
logger.log(Level.WARN,
"Service changecount update for {0} had a problem", e,
registration.getReference());
}
}
Expand Down
3 changes: 1 addition & 2 deletions scr/src/main/java/org/apache/felix/scr/impl/helper/ConfigAdminTracker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ public RegionConfigurationSupport addingService(ServiceReference<ConfigurationAd
catch ( final Exception ex)
{
componentActivator.getLogger().log(Level.ERROR,
"Configuration admin API visible to bundle " + componentActivator.getBundleContext().getBundle() +
" is not the same as the Configuration Admin API visible to the SCR implementation.", ex);
"Configuration admin API visible to bundle {0} is not the same as the Configuration Admin API visible to the SCR implementation.", ex, componentActivator.getBundleContext().getBundle());
}

if ( !visible )
Expand Down
6 changes: 3 additions & 3 deletions scr/src/main/java/org/apache/felix/scr/impl/inject/methods/BaseMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private MethodInfo<T> findMethod(final ComponentLogger logger) throws Invocation
// This is 'Debug' however, as the class information, where the search
// was conducted, is quite important for the client's own debugging
logger.log(Level.DEBUG,
"Locating method " + getMethodName() + " within class " + theClass.getName(), null );
"Locating method {0} within class {1}", null, getMethodName(), theClass.getName());
}

while (true)
Expand All @@ -177,7 +177,7 @@ private MethodInfo<T> findMethod(final ComponentLogger logger) throws Invocation
// Trace as this is going through the whole hierachy and will log for non-existing methods
// a lot of lines for e.g. the class Object not containing an activate method
logger.log(Level.TRACE,
"Locating method " + getMethodName() + " in class " + theClass.getName(), null );
"Locating method {0} in class {1}", null, getMethodName(), theClass.getName());
}

try
Expand Down Expand Up @@ -271,7 +271,7 @@ private MethodResult invokeMethod(final Object componentInstance, final P rawPar
catch ( IllegalStateException ise )
{
rawParameter.getComponentContext().getLogger().log(Level.DEBUG,
ise.getMessage(), null);
ise.getMessage(), ise);
return null;
}
catch ( IllegalAccessException ex )
Expand Down
40 changes: 17 additions & 23 deletions scr/src/main/java/org/apache/felix/scr/impl/inject/methods/BindMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG,
"doFindMethod: Looking for method " + targetClass.getName() + "." + getMethodName(), null );
"doFindMethod: Looking for method {0}.{1}", null, targetClass.getName(), getMethodName());
}

// Case 1 - Service reference parameter
Expand All @@ -111,8 +111,7 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
{
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG, "doFindMethod: Found Method " + method,
null);
logger.log(Level.DEBUG, "doFindMethod: Found Method {0}",null, method);
}
return new MethodInfo<>(method, Collections.singletonList(ValueUtils.ValueType.ref_serviceReference));
}
Expand All @@ -132,8 +131,7 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
{
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG, "doFindMethod: Found Method " + method,
null);
logger.log(Level.DEBUG, "doFindMethod: Found Method {0}", null, method);
}
return new MethodInfo<>(method, Collections.singletonList(ValueUtils.ValueType.ref_serviceObjects));
}
Expand All @@ -153,8 +151,8 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
{
logger.log(
Level.DEBUG,
"doFindMethod: No method taking ServiceReference found, checking method taking "
+ parameterClass.getName(), null );
"doFindMethod: No method taking ServiceReference found, checking method taking {0}",
null, parameterClass.getName() );
}

// Case 3 - Service object parameter
Expand All @@ -165,8 +163,7 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
{
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG, "doFindMethod: Found Method " + method,
null);
logger.log(Level.DEBUG, "doFindMethod: Found Method {0}", null, method);
}
return new MethodInfo<>(method,
Collections.singletonList(ValueUtils.ValueType.ref_serviceType));
Expand All @@ -185,8 +182,7 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
{
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG, "doFindMethod: Found Method " + method,
null);
logger.log(Level.DEBUG, "doFindMethod: Found Method {0}", null, method);
}
return new MethodInfo<>(method,
Collections.singletonList(ValueUtils.ValueType.ref_serviceType));
Expand All @@ -208,7 +204,7 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG,
"doFindMethod: Found Method " + method, null);
"doFindMethod: Found Method {0}", null, method);
}
return new MethodInfo<>(method,
Collections.singletonList(ValueUtils.ValueType.ref_map));
Expand All @@ -233,7 +229,7 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG,
"doFindMethod: Found Method " + method, null);
"doFindMethod: Found Method {0}", null, method);
}
List<ValueUtils.ValueType> paramTypes = new ArrayList<>(2);
paramTypes.add(ValueUtils.ValueType.ref_serviceType);
Expand All @@ -256,7 +252,7 @@ protected MethodInfo<List<ValueUtils.ValueType>> doFindMethod( final Class<?> ta
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG,
"doFindMethod: Found Method " + method, null);
"doFindMethod: Found Method {0}", null, method);
}
List<ValueUtils.ValueType> paramTypes = new ArrayList<>(2);
paramTypes.add(ValueUtils.ValueType.ref_serviceType);
Expand Down Expand Up @@ -352,7 +348,7 @@ else if ( paramType.getName().equals(ClassUtils.FORMATTER_LOGGER_CLASS) )
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG,
"doFindMethod: Found Method " + m, null);
"doFindMethod: Found Method {0}", null, m);
}
return new MethodInfo<>(m, paramTypes);
}
Expand All @@ -366,8 +362,7 @@ else if (logger.isLogEnabled(Level.WARN))
{
logger.log(
Level.WARN,
"doFindMethod: Cannot check for methods taking parameter class " + m_referenceClassName + ": "
+ targetClass.getName() + " does not see it", null );
"doFindMethod: Cannot check for methods taking parameter class {0}: {1} does not see it", null, m_referenceClassName, targetClass.getName());
}

// if at least one suitable method could be found but none of
Expand Down Expand Up @@ -481,8 +476,7 @@ private Method getServiceObjectAssignableMethod( final Class<?> targetClass, fin
{
logger.log(
Level.DEBUG,
"getServiceObjectAssignableMethod: Checking " + candidateBindMethods.length
+ " declared method in class " + targetClass.getName(), null );
"getServiceObjectAssignableMethod: Checking {0} declared method in class {1}", null, candidateBindMethods.length, targetClass.getName());
}

// Iterate over them
Expand All @@ -492,7 +486,7 @@ private Method getServiceObjectAssignableMethod( final Class<?> targetClass, fin
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG,
"getServiceObjectAssignableMethod: Checking " + method, null);
"getServiceObjectAssignableMethod: Checking {0}", null, method);
}

// Get the parameters for the current method
Expand All @@ -507,7 +501,7 @@ private Method getServiceObjectAssignableMethod( final Class<?> targetClass, fin
if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(Level.DEBUG,
"getServiceObjectAssignableMethod: Considering " + method, null);
"getServiceObjectAssignableMethod: Considering {0}", null, method);
}

// Get the parameter type
Expand All @@ -530,8 +524,8 @@ else if (logger.isLogEnabled(Level.DEBUG))
{
logger.log(
Level.DEBUG,
"getServiceObjectAssignableMethod: Parameter failure: Required " + theParameter + "; actual "
+ parameterClass.getName(), null );
"getServiceObjectAssignableMethod: Parameter failure: Required {0}; actual {1}",
null, theParameter, parameterClass.getName());
}

}
Expand Down
Loading
Loading