apache / accumulo

Apache Accumulo
https://accumulo.apache.org
Apache License 2.0
1.06k stars 445 forks source link

RFileScanner's IteratorEnvironment throws UnsupportedOperationException for getConfig() and getServiceEnv() #4810

Open FineAndDandy opened 4 weeks ago

FineAndDandy commented 4 weeks ago

Describe the bug When attaching an iterator to an RFileScanner which uses either the iterator environments config or ServiceEnvironment, the iterators will throw UnsupportedOperationExceptions

Versions (OS, Maven, Java, and others, as appropriate): Accumulo 2.x

To Reproduce Steps to reproduce the behavior (or a link to an example repository that reproduces the problem):

  1. Create an RFileScanner
  2. Attach an Iterator which in the init method calls env.getConfig() or env.getServiceEnv()

Expected behavior The config for the RFileScanner should be built from any table properties passed into the builder's withTableProperties() method. These properties can also be returned for any env.getServiceEnv().getConfiguration() or env.getServiceEnv().getConfiguration(TableId) methods.

kevinrr888 commented 4 weeks ago

These methods have both been deprecated (getConfig() since 2.0.0 and getServiceEnv() since 2.1.0) and replaced with getPluginEnv() (since 2.1.0).

The equivalent to env.getServiceEnv() would now be env.getPluginEnv()

and the equivalent to env.getConfig() would now be env.getPluginEnv().getConfiguration(env.getTableId())

Hope this helps!

ctubbsii commented 4 weeks ago

@kevinrr888 are you saying that using the replacement APIs fixes the issue, or are you just adding extra context to whoever works on fixing the bug?

FineAndDandy commented 4 weeks ago

@kevinrr888 using getPluginEnv() under the covers calls getServiceEnv(), leading to the same UnsupportedOperationException. The issue is the RFileScanner uses an IteratorEnvironment implementation which implements none of these methods.

kevinrr888 commented 4 weeks ago

@FineAndDandy

Yes, you are right

I saw that getPluginEnv() just called getServiceEnv() but tested (or so I thought) the RFileScanner and it was working for getPluginEnv() and getServiceEnv(). I assumed the IteratorEnvironment used for RFileScanner had these methods implemented. Upon looking and testing again, I see the same problem as you.

Looking into this and potentially more issues with IteratorEnvironment and its implemenations...

@ctubbsii

I thought this replacement fixed the issue, but now see that this is not the case

FineAndDandy commented 4 weeks ago

OfflineIteratorEnvironment has similar issues

kevinrr888 commented 3 weeks ago

@FineAndDandy - The issue with OfflineIteratorEnvironment should be fixed in the latest release - 2.1.3 (which was very recently released). Are you on an earlier version?

ddanielr commented 3 weeks ago

https://github.com/apache/accumulo/pull/4283 Related code changes are here.