ACCUMULO-4318 Made writers and scanners auto closeable

[keith-turner]

No description provided.

[dhutchis]

One thought--- a client might open a BatchScanner in a try-with-resources block but save a reference to scanner.iterator() and use that outside the block. If so the client will fail because the BatchScanner will close the query thread pool that the TabletServerBatchReader uses. It would be good to advise users in the documents not to do this.

[joshelser]

That's a very good point, @dhutchis. I think I've seen really weird things in the same manner where a BatchScanner falls out of scope, the finalizer closes it, and then invalidates the iterator being iterated over (in a for-each loop). I think @wjsl also hit this before.

[keith-turner]

It would be good to advise users in the documents not to do this.

I can add some javadoc on the close methods.

[joshelser]

Noticed that you also added some closing of Scanners that were left to auto-close on GC

[keith-turner]

Noticed that you also added some closing of Scanners that were left to auto-close on GC

No exactly sure what you mean. Can you point it out with a comment?

[joshelser]

No exactly sure what you mean. Can you point it out with a comment?

Maybe close by GC is wrong (I forget if we actually have a finalizer on it now). But, anyways, you added some close calls on Scanners :)

https://github.com/apache/accumulo/pull/104/files#diff-34221dcf488e3b53687222557d82b2f1R300

[keith-turner]

Maybe close by GC is wrong (I forget if we actually have a finalizer on it now). But, anyways, you added some close calls on Scanners :)

ok. Introducing Autocloseable introduced lots of warnings in the code. I was just trying to suppress the warnings. When I did this I was thinking Scanners don't have to be closed, so don't need to use try-with-resources. However this could change in the future and the warnings will not be there. I'll change those to use try-with-resources.

[keith-turner]

Merged this to 1.8. Thanks @joshelser and @dhutchis for taking a look.