cshannon commented on PR #5284: URL: https://github.com/apache/accumulo/pull/5284#issuecomment-2614044877
@keith-turner - I had a couple follow up questions/thoughts on the changes here: 1. I believe most of the code assumes the `TabletMergeability` column always exists, but I'm not sure if we should be handling it missing. I don't recall if we talked about adding it on upgrade or not if not set. 2. Should we allow setting a negative delay? A negative delay is confusing but would have the side effect of allowing a tablet to merge for a period of time (say the next 2 days) and then wouldn't allow it after that. I don't know that there is a use case for that but it could be supported because we are using an explicit "never" flag for the metadata and in the RPC calls so "never" isn't tied to -1 or a negative delay. 3. Do you have any ideas on how to better test the API for retrieving `TabletMergeabilityInfo` on errors? The logic will retry splits if there are errors but I couldn't think of a great way to test it, at least with integration tests. Maybe we need something with mocking. 4. I used a supplier to delay retrieving the current system time (it's shared across all the tablets that are part of the getTabletInformation() query) in case the information isn't required and to delay the current time until as late as possible and then it will be cached. We could get rid of this and just compute it and set it on retrieval. -- 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]
