Compactor should use ZooCache to get manager address

[keith-turner]

Each time the compactor gets its next compaction job it goes to ZK to get the manager address. This could be changed to use zoocache which would put less load on ZK.

The following code goes to ZK directly to get the manager address.

accumulo/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java

Line 462 in 8cd7c59

var coordinatorHost = ExternalCompactionUtil.findCompactionCoordinator(getContext());

Saw this issue in elasticity branch, did not check if 2.1 and main branches have the same behavior.

[ctubbsii]

I was looking at code adjacent to this recently, and 2.1 and the main branches also go to ZK directly, because they are going through the ServiceLock APIs in the same way as the elasticity branch. So, I don't think this is an elasticity-specific issue.

One thing to note, though, from the title of this issue, is that it's not really trying to get the "manager address", so much as it is trying to get the address of the coordinator service. That just happens to be running in the manager for the elasticity branch. But, the same basic principle would still apply to the earlier branches where the compaction coordinator is its own process. There are some other differences, too... 2.1 uses an older iteration of the ServiceLock abstraction APIs, for example.

I haven't looked at ZooCache recently, but I do know that for all its benefits, it's is kind of a pain to work with. I'd hate to lean on it more, especially if leaning on it means calling code uses it directly, and circumvents the nice abstraction layer we get with the ServiceLockData API (ServiceLock in 2.1). So, maybe if there's some kind of caching supported, it should be built into the ServiceLockData API, so calling code doesn't need to deal with ZooCache directly. ZooCache does seem like it'd be a little more of a low-level operation than the higher level abstraction we get with ServiceLockData.

The description of this issue actually sounds a bit more like the tablet location cache use case instead of the way we typically use ZooCache. I wonder if there's some pattern or concept we can take from the tablet location cache and turn it into a service location cache. For example, if you try to connect to a service at a cached address, and it's no longer running there, then that's when you expire the cache and look up the address again.

Also, I'm generally curious, how much of a problem is this? Are these ServiceLockData lookups in ZK that expensive?

[dlmarion]

The change in #3813 was pretty simple considering ClientContext was already being passed to the ExternalCompactionUtil method.

[ctubbsii]

The change in #3813 was pretty simple considering ClientContext was already being passed to the ExternalCompactionUtil method.

Yeah, I didn't realize ServiceLock already was designed to support using ZooCache, and that part didn't even change across versions. So, pretty much everything I said before was completely moot. 🤷 😺