milleruntime commented on issue #2968: URL: https://github.com/apache/accumulo/issues/2968#issuecomment-1259969709
I think there might be a bug in ReadOnlyStore, in that it assumes a call to `top()` in it's underlying store (ZooStore) can't be null. This can be seen in 1.10: https://github.com/apache/accumulo/blob/5363d781d3c7f304765c1bdef93969e84f1d934f/fate/src/main/java/org/apache/accumulo/fate/ReadOnlyStore.java#L89 The ReadOnlyRepoWrapper constructor above requires that the Repo returned from the call to `top()` be non-null. But the impl code in ZooStore, clearly has code to handle the case when `top()` returns null: https://github.com/apache/accumulo/blob/b11a03615d583a3f7cc6bebd975d98d3552b83cd/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java#L257-L259 ReadOnlyStore was only used in limited places (deprecated FateAdmin and upgrade code) in previous versions so its likely this went unnoticed. For some reason, we started using ReadOnlyStore in other places in the Fate code and I am starting to think that this was a bad idea. I have one fix for this in https://github.com/apache/accumulo/pull/2971 but there are other places in the code that it is used that should be addressed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
