HDDS-14730. Update Recon container sync to use container IDs

[jasonosullivan34]

What changes were proposed in this pull request?

• Change to use Container IDs to reduce the payload size for the RPCs between Recon and SCM for container sync

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14730

How was this patch tested?

Unit tests
Manual testing

@devmadhuu

[devmadhuu]

Thanks @jasonosullivan34 for the patch. Few comments in line with code. Pls check.

Also please add in PR description , how the patch was tested. Also better write following tests:

• A unit test for ContainerStateMap.getContainerIDs(state, start, count) verifying pagination and state filtering

• A unit or integration test for syncWithSCMContainerInfo() covering the "container missing from Recon, add it" path, and the "container already present, skip it" path

[devmadhuu]

ContainerStateMap.getContainerIDs() uses tailMap(start) which is inclusive. Pls correct the javadoc.

[jasonosullivan34]

I copied this javadoc from ContainerManager.getContainers, will I correct that one too?

[devmadhuu]

Yes, should be corrected

[devmadhuu]

Thanks @jasonosullivan34 for improving the patch, however, still few points need to handle. Pls check.

[devmadhuu]

Suggested change

	if (isSyncDataFromSCMRunning.compareAndSet(false, true)) {
	if (isSyncDataFromSCMRunning.compareAndSet(false, true)) {
	try {
	return containerSyncHelper.syncWithSCMContainerInfo();
	} finally {
	isSyncDataFromSCMRunning.compareAndSet(true, false);
	}
	}
	LOG.debug("SCM DB sync is already running.");
	return false;

[devmadhuu]

Earlier, CONTAINER_METADATA_SIZE was defined as 1MB to estimate a ContainerInfo object. A ContainerID proto is ~8–12 bytes. With an IPC max of 128MB, the old code limited batches to floor(128MB / 1MB) = 128 containers per call. The new code does the same — 128 IDs per call — when it could safely fetch floor(128MB / 12 bytes) ≈ 5.5 million IDs per call. This means the change may actually increase the number of RPCs instead of reducing them. So I think, we should test with large set of container Ids specially when a cluster will have 4-5 millions CLOSED containers and record the SCM latency and impact. Because that was the whole objective behind why this JIRA was raised. And based on impact, we should think of innovative way to handle impact on SCM as well as rpc message length also should not exceed default 128 MB.

[jasonosullivan34]

I propose limiting the number of container IDs we fetch back in a single message to 500K. This would equate to 16MB which is well within the RPC message size limit and should be safe enough for SCM to handle.

I also want to introduce a config where we can reduce the number of container ids fetched if we need to fine tune due to memory constraints

[devmadhuu]

The helper already catches all Exception internally and returns false — so the throws Exception is misleading. The facade should declare throws IOException (or nothing if the helper swallows everything).