search

NationalSecurityAgency / datawave

DataWave is an ingest/query framework that leverages Apache Accumulo to provide fast, secure data access.
https://code.nsa.gov/datawave
Apache License 2.0
561 stars 243 forks source link

Replace nonpublic Accumulo IterationInterruptedException #2539

Open SethSmucker opened 1 month ago

SethSmucker commented 1 month ago

IterationInterruptedException is not part of Accumulo's public API.

Create IterationInterruptedException class in DataWave, replacing Accumulo's version.

part of https://github.com/NationalSecurityAgency/datawave/issues/2443

SethSmucker commented 3 weeks ago

See Adam's comment

Is it still too close? I changed it from IterationInterruptedException to IterationInterruptException. Might be a bit confusing if they're this close, what do you think?

ivakegg commented 3 weeks ago

See Adam's comment

Is it still too close? I changed it from IterationInterruptedException to IterationInterruptException. Might be a bit confusing if they're this close, what do you think?

Now that I am looking one layer deeper, I realize that we were throwing that exception because we are implementing InterruptibleIterator which is an accumulo class. Is that class part of the public API? If so then we need to push for the original IteratorInterruptedException to be part of the public api. Can you push that question over to the accumulo group please?

ddanielr commented 3 weeks ago

See Adam's comment

Is it still too close? I changed it from IterationInterruptedException to IterationInterruptException. Might be a bit confusing if they're this close, what do you think?

Now that I am looking one layer deeper, I realize that we were throwing that exception because we are implementing InterruptibleIterator which is an accumulo class. Is that class part of the public API? If so then we need to push for the original IteratorInterruptedException to be part of the public api. Can you push that question over to the accumulo group please?

@ivakegg the InterruptibleIterator is not in the list of iterators in the accumulo public API After looking through the code, there are two datawave iterators which implement InterruptibleIterator, SortedMultiMapIterator and ColumnRangeIterator.

SortedMultiMapIterator is used for a test case so it could probably be modified to implement a public iterator class without too much impact to other code behavior. However ColumnRangeIterator might need further review to determine the next steps.

ivakegg commented 2 weeks ago

Ok, it appears that whole mechanism is basically OBE. So essentially, everywhere we were passing InterationInterruptedException should remain as is. We simply should not be implementing the InterruptibleIterator and hence should not have any flag to check on. @SethSmucker Please move that whole mechanism, and keep the places where we are passing the InterationInterruptedException back up the stack.