Thanks Sangjin Lee for the review! Here are some answers:
We need a different way to get at the location of the jar (coming from the maven local cache).
Yes. I wanted to load the coprocessor jar from classpath but I am not certain how to do that in the unit test on the minicluster. We can discuss further on this.
Question: I see we're making a change to upload and use the jar that has the co-processor from hdfs, rather than adding it to the hbase install. Is that change needed for the dynamic co-processor?
Yes, dynamic loading API requires the coprocessor jar to be present on HDFS. The Region Server does not need a restart when dynamic loading is used.
What is the benefit of doing that over using it out of the local install?
The local install would mean a restart of the region server but this is what is called as static loading. When coprocessors are loaded this way, they are turned on for all tables. We don't want that. We would like the coprocessor to be enabled for only the flow run table, hence dynamic loading is the way to go.
I always wanted to do dynamic loading but we could not use it since the previous version of hbase was not allowing system classes to be loaded as dynamically loaded coprocessors. So it's not a new thought, it was always the intention but was not possible due to hbase version dependency.
maybe I'm bit on the paranoid side, but checking whether it is operating on the right table seems like a reasonable sanity check nonetheless; do we need to remove this check?
Keeping the check won't break anything. But it will slow it down. Since this is a table level coprocessor, the check is unnecessary and at high loads, the performance would likely be affected.
Do we need to move the package for this and TestFlowDataGenerator? I'm curious as to why that move is necessary. The diffs are now deleting the old files and creating the new files so it's not so easy to see what has changed.
These two classes DataGeneratorForTest and TestFlowDataGenerator are common to most of the unit tests and they should have been put in a common package but for some reason, we did not do that earlier. This move feels correct to me.
Regarding the diffs, I understand the changes are not easily understandable since the files moved. I think I can upload a diff of the two files to make it easier for review, let me know.