keith-turner commented on PR #4746: URL: https://github.com/apache/accumulo/pull/4746#issuecomment-2244069615
> I had the changes in https://github.com/apache/accumulo/commit/6f1303b656c9c3746031fbd51a39329038ef2753 staged before I read your comment. So I pushed them, but can change them if you like. I just made a note in the javadoc that if the converter returns null the previous value is used. I don't have any recommendations at the moment. Was poking around in the code trying to understand what is going on. Looking in the code found that pattern that the scan and compaction dispatchers on failure will fall back to the default services, like the following code. https://github.com/apache/accumulo/blob/0e7417d0369aca85e149563cc52ad7c9108dfe6d/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java#L1520-L1525 So it seems some code was relying on the nulls for compaction, but maybe not for scans not sure. The following were some situations I was thinking through when looking at the code. * User configures an invalid class name for a compaction dispatcher or scan dispatcher. In this case, I think the code before and after the changes in this PR would probably fall back to the default services/executors. * User configures a valid class name for a dispatcher, however there is a transient classloader problem where the class can not be created. Not sure about what would happen before these changes. After these changes the code may stick with the default impls that were passed to Property.createTableInstanceFromPropertyName even after the transient class loader issue resolves itself. * User configures a valid class name for a dispatcher and the class is loaded ok, but it throws an error in init. I think before and after these changes this will cause an exception to percolate up. So its seems the code goes through a lot of trouble to handle one error (class loading or class not found) and fall back to defaults, but for another type of error it just lets it fly. Based on this https://github.com/apache/accumulo/issues/4423#issuecomment-2243412557 I was only looking at the scan and compaction dispatch when analyzing the code. -- 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]
