Skip to content

[BUG] mgmt, thread stuck when initializing AzureResourceManager instances concurrently #47284

@XiaofeiCao

Description

@XiaofeiCao

Issue is from a customer-reported IcM, where the code change from reading properties file(e.g. azure-resourcemanager-compute.properties) in static block to reading per AzureServiceClient instance:
c10f1cb#diff-97f54a26a4f9e2108ccfce7b3e87fd5e94b7fd57d1a8804771105cb3e8e935f1

The change is due to the fact that azure-resourcemanager packages are now separated, and each package has its independent version. We cannot use the unified version of azure-resourcemanager-resources anymore.
Though the actual implementation is no good. Now it reads from disk for every AzureServiceClient instances. It may not be noticeable for a single AzureResourceManager, but very much for multiple ones(1000+).

We want to limit the time needed to read from disk, while still be able to correctly identify the library version of the actual service client.

There are basically 3 timings to load the cache:

  1. Class initialization phase. This is when each AzureServiceClient subclasses get loaded by classloaders, which is what we originally did.
  2. Class instantiation phase. This is when each AzureServiceClient got instantiated through, e.g. new xx(). This is current buggy behavior.
  3. Method call phase. This is like lazy-loading and only load cache when it got a cache miss doing actual method calls.

Those will take us to proposals.

Proposals

Option 1 (Class initialization phase)

Register library version in subclasses static block.
This is would require changes to codegen, to do a registration in every AzureServiceClient subclass.

public abstract class AzureServiceClient {
  protected static final Map<Class<? extends AzureServiceClient, String> VERSION_MAP;

}

public class VirtualMachinesClientImpl extends AzureServiceClient {
  static {
    VERSION_MAP.computeIfAbsent(getClass(), readFromFile());
  }
}

Option 2 (Class initialization phase)

Iterate known premium libraries, and register them all in AzureServiceClient.

public class AzureServiceClient {
  private static final Set<String> PROPERTY_FILES; // hard-coded in code, or in a new file
  private static final Map<String, String> VERSION_MAP;
  static {
    for (String propertyFile : PROPERTY_FILES) {
      try {
         String version = readFromFile(propertyFile);
         String sdkName = getSdkName(propertyFile);
         VERSION_MAP.put(sdkName, version);
      } catch(Exception e){
        // NO-OP, ignore the exception
        // in case user uses independent libraries other than azure-resourcemanager
    }
    }
  }
}

Option 3 (Class instantiation phase)

This is done in PR:
#47283

public class AzureServiceClient {
  private static final Map<Class<? extends AzureServiceClient>, String> SDK_VERSION_MAP;
  public AzureServiceClient() {
    readSdkVersionFromPropertiesFileIfAbsent(this.getClass());
  }
}

The PR is done before option2 comes up(shame). It can reduce the disk access need by a great amount, but not guarenteed to solve the thread block.

Option 4(Method call phase)

This one is probably not worth considering. Method calls are much more frequent than class instantiations.

Previously my personal preference is option 3. Now I lean more towards option 2...

Metadata

Metadata

Assignees

Labels

MgmtThis issue is related to a management-plane library.

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions