SDK Java Jersey 2.4 - Client Side evaluation questions / Performance issues

[MarcRoser]

Hi,

I'm not sure if it's a issue or not so i prefer to start a new discussion.

I use the SDK Java Jersey 2.4 with Spring Boot. I've added a bean FeatureHubConfig and a "requestScope" bean ClientContext created with featureHubConfig.newContext() (with the userKey, version, ...).

When "FeatureHubConfig" is initialized, a newContext is built : (FeatureHub SDK sources)

  @Override
  public void init() {
    try {
      final Future<ClientContext> futureContext = newContext().build();
      futureContext.get();
    } catch (Exception e) {
      log.error("Failed to initialize FeatureHub client", e);
    }
  }

With this new context, the repository and a new JerseyClient are created : (still FeatureHub SDK sources)

  public ClientContext newContext(@Nullable FeatureRepositoryContext repository,
                                  @Nullable Supplier<EdgeService> edgeService) {
//...
      this.repository = new ClientFeatureRepository();
      repository = this.repository;
//...
      this.edgeService = loadEdgeService(repository); // JerseyFeatureHubClientFactory createEdgeService
      edgeService = this.edgeService;
//...

    return new ClientEvalFeatureContext(this, repository, edgeService.get());
  }

public class JerseyFeatureHubClientFactory implements FeatureHubClientFactory {
  @Override
  public Supplier<EdgeService> createEdgeService(FeatureHubConfig config, FeatureStore repository) {
    return () -> new JerseyClient(config, repository);
  }
}

Everything is ok. The JerseyClient start to listen and the repository is populated.

Then, for every request, a new context is created and with the supplier edgeService.get(), for each context, a new JerseyClient is created with () -> new JerseyClient(config, repository) even if every ClientEvalFeatureContext work with the same repository.

Each JerseyClient start its own requests and listen in order to update the same repository.

At the end of every request, the "requestScope" bean ClientContext is closed. JerseyClient.close/shutdown() is called and if some times have passed and the eventInput is filled, the request is stucked several seconds on eventInput.close() (if not, the JerseyClient is created, closed and it does nothing).

Should the JerseyClient be a singleton ? (i can use FeatureHubConfig::setEdgeService for that but it may be a bug)
If the JerseyClient is provided and is a singleton, ClientEvalFeatureContext must not close it.

Some commentaries aren't coherent : So the edge service stays the same.. if we created the edge :

/**
 * This class is ONLY used when we are doing client side evaluation. So the edge service stays the same.
 */
class ClientEvalFeatureContext extends BaseClientContext {

  /**
   * We only close if we created the edge and repository
   */
  @Override
  public void close() {
    if (weOwnRepositoryAndEdge) {
      repository.close();
    }
    edgeService.close();
  }
}

Why edgeService.close(); ?

Am I not understanding something ? It works very well with a single unclosabled JerseyClient i provide but i may overrided an intended behavor and i feel something is wrong...

Thanks

[rvowles]

Hey there!

The source of the newConext in your example is missing some critical checks:

`
public ClientContext newContext(@nullable FeatureRepositoryContext repository,
@nullable Supplier edgeService) {
if (repository == null) {
if (this.repository == null) {
this.repository = new ClientFeatureRepository();
}

  repository = this.repository;
}

if (edgeService == null) {
  if (this.edgeService == null) {
    this.edgeService = loadEdgeService(repository);
  }

  edgeService = this.edgeService;
}

`

If null is passed in both cases (as it is for newContext().build()), it is checking if the edge and repository already exist on the FHClient, so it won't create them again, unless I am missing a logic bomb of some kind?

I think your comment about closing the context at the end of the requestScoped bean is the confusion here. The close actually doing anything should only be on the ServerEvaluatedFeatureContext, you should not call it when you are using client-evaluated (for your Spring Boot server). I think the "weOwnRepositoryAndEdge" was me thinking it was the only connection you had and you were using client evaluated features, but thats pretty unlikely, so I should just make that close method do nothing.

Apologies for the confusion!

[MarcRoser]

Hi,

Thanks for the fast answer !

With if (this.edgeService != null), It will not create the supplier again. But it will create another JerseyClient with this.edgeService.get(). That's the "logic bomb".

this.edgeService = loadEdgeService(repository); is the same as this.edgeService = () -> new JerseyClient(config, repository);

Thus the line return new ClientEvalFeatureContext(this, repository, edgeService.get()); is the same as return new ClientEvalFeatureContext(this, repository, new JerseyClient(config, repository));

You can easily check it with a breakpoint in the constructor.

Since ClientEvalFeatureContext is always created with a new JerseyClient (it seems like a bug), it's normal to close it.
In fact, the method close is called by Spring Boot (2.7.1) when it destroy the beans (requestScope) with its DisposableBeanAdapter. But i can override the close to make it do nothing (it's what i've done...). I don't call it explicitly.

java reflection + ... in :

package org.springframework.beans.factory.support;

class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {

	private static final String DESTROY_METHOD_NAME = "destroy";

	private static final String CLOSE_METHOD_NAME = "close"; // it will find the close method of the bean ClientContext 

	private static final String SHUTDOWN_METHOD_NAME = "shutdown";
//...
}

    @Bean
    @Scope(value = WebApplicationContext.SCOPE_REQUEST, proxyMode = ScopedProxyMode.TARGET_CLASS)
    ClientContext clientContext(FeatureHubConfig fhConfig) {
        ClientContext fhClient = fhConfig.newContext();
        //...
        return fhClient;
    }

I hope it will help.

[rvowles]

Took me a while to find this again! I totally get you and on the other SDKs, I haven't made this mistake 🤦 - there are a few issues i'll fix over this coming weekend and this is one of them. If you could like to track it, you could open this as a bug in that repo?

[MarcRoser]

Ok, i'll do it.

Thanks

[MarcRoser]

https://github.com/featurehub-io/featurehub/issues/806