Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion merged PR #5490:
URL: https://github.com/apache/accumulo/pull/5490


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2828375402

   @keith-turner @kevinrr888 - Thanks for all your reviews. I'm going to work 
on merging this in and up to main, then I'll create the follow-on issues.


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2058935992


##
core/src/test/java/org/apache/accumulo/core/iterators/DefaultIteratorEnvironment.java:
##
@@ -1,56 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.accumulo.core.iterators;
-
-import java.io.IOException;
-
-import org.apache.accumulo.core.conf.AccumuloConfiguration;
-import org.apache.accumulo.core.conf.DefaultConfiguration;
-import org.apache.accumulo.core.data.Key;
-import org.apache.accumulo.core.data.Value;
-import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.FileSystem;
-
-public class DefaultIteratorEnvironment implements IteratorEnvironment {
-
-  AccumuloConfiguration conf;
-  Configuration hadoopConf = new Configuration();
-
-  public DefaultIteratorEnvironment(AccumuloConfiguration conf) {
-this.conf = conf;
-  }
-
-  public DefaultIteratorEnvironment() {
-this.conf = DefaultConfiguration.getInstance();
-  }
-
-  @Deprecated(since = "2.0.0")
-  @Override
-  public SortedKeyValueIterator reserveMapFileReader(String 
mapFileName)
-  throws IOException {
-FileSystem fs = FileSystem.get(hadoopConf);
-return new MapFileIterator(fs, mapFileName, hadoopConf);
-  }

Review Comment:
   The ITs passed with this change, so I think we are safe. IIRC, Keith said 
above that he could not find any uses of it.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

kevinrr888 commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2058713933


##
core/src/test/java/org/apache/accumulo/core/iterators/DefaultIteratorEnvironment.java:
##
@@ -1,56 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.accumulo.core.iterators;
-
-import java.io.IOException;
-
-import org.apache.accumulo.core.conf.AccumuloConfiguration;
-import org.apache.accumulo.core.conf.DefaultConfiguration;
-import org.apache.accumulo.core.data.Key;
-import org.apache.accumulo.core.data.Value;
-import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.FileSystem;
-
-public class DefaultIteratorEnvironment implements IteratorEnvironment {
-
-  AccumuloConfiguration conf;
-  Configuration hadoopConf = new Configuration();
-
-  public DefaultIteratorEnvironment(AccumuloConfiguration conf) {
-this.conf = conf;
-  }
-
-  public DefaultIteratorEnvironment() {
-this.conf = DefaultConfiguration.getInstance();
-  }
-
-  @Deprecated(since = "2.0.0")
-  @Override
-  public SortedKeyValueIterator reserveMapFileReader(String 
mapFileName)
-  throws IOException {
-FileSystem fs = FileSystem.get(hadoopConf);
-return new MapFileIterator(fs, mapFileName, hadoopConf);
-  }

Review Comment:
   I may be missing something, but seems like the replacement `public static 
final IteratorEnvironment DEFAULT = new Builder().build();` for this class will 
`throw new UnsupportedOperationException("Feature not supported");` for this 
method. If DEFAULT is only used in testing this probably doesn't matter so long 
as everything still passes



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2058552902


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -74,8 +78,46 @@
 
 class RFileScanner extends ScannerOptions implements Scanner {
 
+  private static class RFileScannerEnvironmentImpl extends 
ClientServiceEnvironmentImpl {
+
+private final Configuration conf;
+private final Configuration tableConf;
+
+public RFileScannerEnvironmentImpl(Opts opts) {
+  super(null);
+  conf = new ConfigurationImpl(new 
ConfigurationCopy(DefaultConfiguration.getInstance()));
+  ConfigurationCopy tableCC = new 
ConfigurationCopy(DefaultConfiguration.getInstance());
+  if (opts.tableConfig != null) {
+opts.tableConfig.forEach(tableCC::set);
+  }
+  tableConf = new ConfigurationImpl(tableCC);
+}
+
+@Override
+public String getTableName(TableId tableId) throws TableNotFoundException {
+  Preconditions.checkArgument(tableId == TABLE_ID, "Expected " + TABLE_ID 
+ " obtained"
+  + " from IteratorEnvironment.getTableId(), but got: " + tableId);
+  return TABLE_NAME;
+}
+
+@Override
+public Configuration getConfiguration() {
+  return conf;
+}
+
+@Override
+public Configuration getConfiguration(TableId tableId) {
+  Preconditions.checkArgument(tableId == TABLE_ID, "Expected " + TABLE_ID 
+ " obtained"
+  + " from IteratorEnvironment.getTableId(), but got: " + tableId);
+  return tableConf;
+}
+
+  }
+
   private static final byte[] EMPTY_BYTES = new byte[0];
   private static final Range EMPTY_RANGE = new Range();
+  private static final String TABLE_NAME = "rfileScanner";
+  private static final TableId TABLE_ID = TableId.of(TABLE_NAME);

Review Comment:
   Could spin this off in a separate issue since it was a preexisting pattern 
in 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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2058547999


##
server/base/src/main/java/org/apache/accumulo/server/iterators/TabletIteratorEnvironment.java:
##
@@ -79,6 +80,30 @@ public TabletIteratorEnvironment(ServerContext context, 
IteratorScope scope,
 this.topLevelIterators = new ArrayList<>();
   }
 
+  public TabletIteratorEnvironment(ServerContext context, IteratorScope scope,

Review Comment:
   I experimented w/ using the new code in InMemoryMapTest to create an iterenv 
and it worked really nicely and the test passed.   The new class is a good 
general purpose builder for an IteratorEnv, seems fine to me to use it in the 
server code and remove this new constructor.
   
   ```java
 IteratorEnvironment iterEnv =
 new 
ClientIteratorEnvironment.Builder().withClient(getServerContext())
 
.withScope(IteratorScope.scan).withTableId(TableId.of("foo")).build();
 IteratorEnvironment iterEnvSC =
 new 
ClientIteratorEnvironment.Builder().withClient(getServerContext())
 
.withScope(IteratorScope.scan).withTableId(TableId.of("foo")).withSamplingEnabled()
 
.withSamplerConfiguration(sampleConfig.toSamplerConfiguration()).build();
 SortedKeyValueIterator iter0dc1 = iter0.deepCopy(iterEnv);
 SortedKeyValueIterator iter0dc2 = iter0.deepCopy(iterEnvSC);
 SortedKeyValueIterator iter1dc1 = iter1.deepCopy(iterEnv);
 SortedKeyValueIterator iter1dc2 = iter1.deepCopy(iterEnvSC);
 SortedKeyValueIterator iter2dc1 = iter2.deepCopy(iterEnv);
 SortedKeyValueIterator iter2dc2 = iter2.deepCopy(iterEnvSC);
   ```
   
   Not a change for this PR, but maybe in the public API I  we cold eventually 
add a static `builder()` method to `IteratorEnvironment` and use this new code 
as the impl.   Then could write the above code like the following.
   
   ```java
 IteratorEnvironment iterEnv =
 IteratorEnvironment.builder().withClient(getServerContext())
 
.withScope(IteratorScope.scan).withTableId(TableId.of("foo")).build();
   ```
   
   In this PR would could add a static `builder()` method to 
ClientIteratorEnvironment. Would make the code that uses the builder slightly 
shorter. 



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2827372395

   Full IT build passed


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056799962


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+private boolean samplingEnabled = false;
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplingEnabled() {
+  this.samplingEnabled = true;
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.ofNullable(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final boolean samplingEnabled;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+this.samplingEnabled = builder.samplingEnabled;
+  }
+
+  /**
+   * Copy constructor used for enabling sample. Only called from {@link 
cloneWithSamplingEnabled}.
+   */
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope;
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths;
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId = copy.tableId;
+this.samplerConfig = copy.samplerCo

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056302590


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+private boolean samplingEnabled = false;
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplingEnabled() {
+  this.samplingEnabled = true;
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.ofNullable(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final boolean samplingEnabled;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+this.samplingEnabled = builder.samplingEnabled;
+  }
+
+  /**
+   * Copy constructor used for enabling sample. Only called from {@link 
cloneWithSamplingEnabled}.
+   */
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope;
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths;
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId = copy.tableId;
+this.samplerConfig = copy.samplerConfig

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2825399925

   I'm not sure about the change I made in 1c75b6c. With the builder pattern 
should we allow the user to call whatever methods they want in which-ever order 
they want?


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

kevinrr888 commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r205678


##
core/src/main/java/org/apache/accumulo/core/iteratorsImpl/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,219 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iteratorsImpl;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorEnvironment;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+private boolean samplingEnabled = false;
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplingEnabled() {
+  this.samplingEnabled = true;
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.ofNullable(sc);
+  return this;
+}
+
+public ClientIteratorEnvironment.Builder 
withEnvironment(ClientServiceEnvironmentImpl env) {
+  this.env = Optional.of(env);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final boolean samplingEnabled;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+this.samplingEnabled = builder.samplingEnabled;
+  }
+
+  /**
+   * Copy constructor used for enabling sample. Only called from {@link 
cloneWithSamplingEnabled}.
+   */
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope;
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths;
+this.isUserCompaction = co

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056877532


##
core/src/main/java/org/apache/accumulo/core/iteratorsImpl/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,219 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iteratorsImpl;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorEnvironment;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+private boolean samplingEnabled = false;
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplingEnabled() {
+  this.samplingEnabled = true;
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.ofNullable(sc);
+  return this;
+}
+
+public ClientIteratorEnvironment.Builder 
withEnvironment(ClientServiceEnvironmentImpl env) {
+  this.env = Optional.of(env);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final boolean samplingEnabled;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+this.samplingEnabled = builder.samplingEnabled;
+  }
+
+  /**
+   * Copy constructor used for enabling sample. Only called from {@link 
cloneWithSamplingEnabled}.
+   */
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope;
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths;
+this.isUserCompaction = copy

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2825457156

   
   > That change is nice for runtime validation. Could add more runtime checks 
making sure things are empty when set in the builder w/ the assumption that if 
the same builder method is called twice it probably is not what was intended. I 
like the way it currently is, the optionals provide a lot of runtime validation 
(like for the case attempting to use something not set in the builder).
   
   Ok, I can add more validation on the Builder
   


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056840098


##
server/base/src/main/java/org/apache/accumulo/server/iterators/TabletIteratorEnvironment.java:
##
@@ -79,6 +80,30 @@ public TabletIteratorEnvironment(ServerContext context, 
IteratorScope scope,
 this.topLevelIterators = new ArrayList<>();
   }
 
+  public TabletIteratorEnvironment(ServerContext context, IteratorScope scope,

Review Comment:
   ah, I think it's because I was trying to get all of the server side code to 
use server-side iterator environment objects vs having all the server side 
tests using the new ClientIteratorEnvironment.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2825447104

   > I'm not sure about the change I made in 
https://github.com/apache/accumulo/commit/1c75b6c646bbd4bf5d7cdfff7d387445ff636445.
 With the builder pattern should we allow the user to call whatever methods 
they want in which-ever order they want?
   
   That change is nice for runtime validation.Could add more runtime checks 
making sure things are empty when set in the builder w/ the assumption that if 
the same builder method is called twice it probably is not what was intended.  
I like the way it currently is, the optionals provide a lot of runtime 
validation (like for the case attempting to use something not set in the 
builder).


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056837386


##
server/base/src/main/java/org/apache/accumulo/server/iterators/TabletIteratorEnvironment.java:
##
@@ -79,6 +80,30 @@ public TabletIteratorEnvironment(ServerContext context, 
IteratorScope scope,
 this.topLevelIterators = new ArrayList<>();
   }
 
+  public TabletIteratorEnvironment(ServerContext context, IteratorScope scope,

Review Comment:
   which test?



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056825666


##
server/base/src/main/java/org/apache/accumulo/server/iterators/TabletIteratorEnvironment.java:
##
@@ -79,6 +80,30 @@ public TabletIteratorEnvironment(ServerContext context, 
IteratorScope scope,
 this.topLevelIterators = new ArrayList<>();
   }
 
+  public TabletIteratorEnvironment(ServerContext context, IteratorScope scope,

Review Comment:
   What is going on w/ this new constructor?  Its only used in a test, what is 
preventing the test from using the new builder?



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056795135


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {

Review Comment:
   Moving out of the public for now seems good, can always move it into API 
later.  Seems like there is potential, but would require some work to figure 
that out.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2825283630

   > @keith-turner - In https://github.com/apache/accumulo/pull/4816, I changed 
the IteratorEnvIT to test across different scanner types. Could use that code 
and expand on that as needed for full test coverage. Could potentially move 
those changes into this PR, or change that PR to only have the changes not 
accomplished by this PR and merge them both in.
   
   After this PR is merged, seems like it would good to circle back to #4816 
and pull the useful bits into another PR.


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

kevinrr888 commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2825257327

   > While reviewing this today I found a place I would like to add a unit test 
for RFileScanner. More generally we may want to eventually have something like 
https://github.com/apache/accumulo/pull/5474 that covers the iteratorEnv in all 
the different scanner types (like 
RFileScanner,OfflineScanner,ScannerImpl,BatchScanner,ClientSideIteratorScanner).
 We could try to as much as possible to run the same tests across all the 
different scanner types. For this PR I will try to run all the unit test w/ 
code coverage and see what the coverage looks like for the new code.
   
   In #4816, I changed the `IteratorEnvIT` to test across different scanner 
types. Could use that code and expand on that as needed for full test coverage. 
Could potentially move those changes into this PR, or change that PR to only 
have the changes not accomplished by this PR and merge them both in.


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056302946


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -74,8 +79,65 @@
 
 class RFileScanner extends ScannerOptions implements Scanner {
 
+  private static class RFileScannerIteratorEnvironmentBuilder
+  extends ClientIteratorEnvironment.Builder {
+
+public ClientIteratorEnvironment.Builder 
withEnvironment(ClientServiceEnvironmentImpl env) {
+  this.env = Optional.of(env);
+  return this;
+}
+
+  }
+
+  private static class RFileScannerEnvironmentImpl extends 
ClientServiceEnvironmentImpl {
+
+private final Opts opts;
+
+public RFileScannerEnvironmentImpl(Opts opts) {
+  super(null);
+  this.opts = opts;
+}
+
+@Override
+public String getTableName(TableId tableId) throws TableNotFoundException {
+  Preconditions.checkArgument(tableId == TABLE_ID, "Expected " + TABLE_ID 
+ " obtained"
+  + " from IteratorEnvironment.getTableId(), but got: " + tableId);
+  return TABLE_NAME;
+}
+
+@Override
+public  T instantiate(String className, Class base)
+throws ReflectiveOperationException, IOException {
+  return 
RFileScanner.class.getClassLoader().loadClass(className).asSubclass(base)
+  .getDeclaredConstructor().newInstance();
+}
+
+@Override
+public  T instantiate(TableId tableId, String className, Class base)
+throws ReflectiveOperationException, IOException {
+  return instantiate(className, base);
+}

Review Comment:
   Removed these in 2d39e46.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056300278


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -74,8 +79,65 @@
 
 class RFileScanner extends ScannerOptions implements Scanner {
 
+  private static class RFileScannerIteratorEnvironmentBuilder
+  extends ClientIteratorEnvironment.Builder {
+
+public ClientIteratorEnvironment.Builder 
withEnvironment(ClientServiceEnvironmentImpl env) {

Review Comment:
   In 2d39e46 I moved ClientIteratorEnvironment out of the public api package, 
and moved this method into it.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056304223


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -74,8 +79,65 @@
 
 class RFileScanner extends ScannerOptions implements Scanner {
 
+  private static class RFileScannerIteratorEnvironmentBuilder
+  extends ClientIteratorEnvironment.Builder {
+
+public ClientIteratorEnvironment.Builder 
withEnvironment(ClientServiceEnvironmentImpl env) {
+  this.env = Optional.of(env);
+  return this;
+}
+
+  }
+
+  private static class RFileScannerEnvironmentImpl extends 
ClientServiceEnvironmentImpl {
+
+private final Opts opts;
+
+public RFileScannerEnvironmentImpl(Opts opts) {
+  super(null);
+  this.opts = opts;
+}
+
+@Override
+public String getTableName(TableId tableId) throws TableNotFoundException {
+  Preconditions.checkArgument(tableId == TABLE_ID, "Expected " + TABLE_ID 
+ " obtained"
+  + " from IteratorEnvironment.getTableId(), but got: " + tableId);
+  return TABLE_NAME;
+}
+
+@Override
+public  T instantiate(String className, Class base)
+throws ReflectiveOperationException, IOException {
+  return 
RFileScanner.class.getClassLoader().loadClass(className).asSubclass(base)
+  .getDeclaredConstructor().newInstance();
+}
+
+@Override
+public  T instantiate(TableId tableId, String className, Class base)
+throws ReflectiveOperationException, IOException {
+  return instantiate(className, base);
+}
+
+@Override
+public Configuration getConfiguration() {
+  return new ConfigurationImpl(new 
ConfigurationCopy(DefaultConfiguration.getInstance()));
+}
+
+@Override
+public Configuration getConfiguration(TableId tableId) {
+  Preconditions.checkArgument(tableId == TABLE_ID, "Expected " + TABLE_ID 
+ " obtained"
+  + " from IteratorEnvironment.getTableId(), but got: " + tableId);
+  ConfigurationCopy tableCC = new 
ConfigurationCopy(DefaultConfiguration.getInstance());
+  opts.tableConfig.forEach(tableCC::set);
+  return new ConfigurationImpl(tableCC);
+}

Review Comment:
   Moved the configuration calculations to the constructor in 2d39e46. Had to 
guard against opts.tableConfig being null, which it is in RFileClientTest.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056301687


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {

Review Comment:
   I moved ClientIteratorEnvironment out of the public API in 2d39e46.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056300894


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+private boolean samplingEnabled = false;
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplingEnabled() {
+  this.samplingEnabled = true;
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.ofNullable(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final boolean samplingEnabled;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+this.samplingEnabled = builder.samplingEnabled;
+  }
+
+  /**
+   * Copy constructor used for enabling sample. Only called from {@link 
cloneWithSamplingEnabled}.
+   */
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope;
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths;
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId = copy.tableId;
+this.samplerConfig = copy.samplerConfig

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056231919


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+private boolean samplingEnabled = false;
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplingEnabled() {
+  this.samplingEnabled = true;
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.ofNullable(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final boolean samplingEnabled;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+this.samplingEnabled = builder.samplingEnabled;
+  }
+
+  /**
+   * Copy constructor used for enabling sample. Only called from {@link 
cloneWithSamplingEnabled}.
+   */
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope;
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths;
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId = copy.tableId;
+this.samplerConfig = copy.samplerConfig

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2056167693


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -74,8 +79,65 @@
 
 class RFileScanner extends ScannerOptions implements Scanner {
 
+  private static class RFileScannerIteratorEnvironmentBuilder
+  extends ClientIteratorEnvironment.Builder {
+
+public ClientIteratorEnvironment.Builder 
withEnvironment(ClientServiceEnvironmentImpl env) {

Review Comment:
   IIRC I had it like this at first and it failed the build because 
`ClientIteratorEnvionment` is in an API package and 
`ClientServiceEnvironmentImpl` is not. This is why 
`ClientIteratorEnvironment.withClient` exists instead.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2822577082

   > Overall, I think these changes are a good idea, however, this refactoring 
can very easily have bugs that are hard to spot. I'm not sure if this is the 
best for a 2.1 change, risking potentially introducing more bugs than the 
original PR set out to fix.
   
   @kevinrr888 I also like these changes and was a bit nervous about making the 
changes in 2.1.  Personally I would like to try to get these changes in 
(because they make things more consistent which will reduce bugs long term, 
also the current inconsistency feels really buggy) and review our test coverage 
around this code.  While reviewing this today I found a place I would like to 
add a unit test for RFileScanner.  More generally we may want to eventually 
have something like #5474 that covers the iteratorEnv in all the different 
scanner types (like 
RFileScanner,OfflineScanner,ScannerImpl,BatchScanner,ClientSideIteratorScanner).
  We could try to as much as possible to run the same tests across all the 
different scanner types.   For this PR I will try to run all the unit test w/ 
code coverage and see what the coverage looks like for the new 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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2054918721


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+private boolean samplingEnabled = false;
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplingEnabled() {
+  this.samplingEnabled = true;
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.ofNullable(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final boolean samplingEnabled;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+this.samplingEnabled = builder.samplingEnabled;
+  }
+
+  /**
+   * Copy constructor used for enabling sample. Only called from {@link 
cloneWithSamplingEnabled}.
+   */
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope;
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths;
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId = copy.tableId;
+this.samplerConfig = copy.samplerCo

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2054584893


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -292,15 +280,54 @@ public Iterator> iterator() {
 EMPTY_BYTES, tableConf);
   }
 
+  ClientServiceEnvironmentImpl senv = new 
ClientServiceEnvironmentImpl(null) {
+
+@Override
+public String getTableName(TableId tableId) throws 
TableNotFoundException {
+  throw new IllegalStateException("TableId not known in RFileScanner");
+}
+
+@Override
+public  T instantiate(String className, Class base)
+throws ReflectiveOperationException, IOException {
+  return 
RFileScanner.class.getClassLoader().loadClass(className).asSubclass(base)
+  .getDeclaredConstructor().newInstance();
+}
+
+@Override
+public  T instantiate(TableId tableId, String className, Class 
base)
+throws ReflectiveOperationException, IOException {
+  return instantiate(className, base);
+}
+
+@Override
+public Configuration getConfiguration() {
+  return new ConfigurationImpl(new 
ConfigurationCopy(DefaultConfiguration.getInstance()));
+}
+
+@Override
+public Configuration getConfiguration(TableId tableId) {
+  return new ConfigurationImpl(new 
ConfigurationCopy(opts.tableConfig));

Review Comment:
   Fixed in bf294a4



##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -292,15 +280,54 @@ public Iterator> iterator() {
 EMPTY_BYTES, tableConf);
   }
 
+  ClientServiceEnvironmentImpl senv = new 
ClientServiceEnvironmentImpl(null) {
+
+@Override
+public String getTableName(TableId tableId) throws 
TableNotFoundException {
+  throw new IllegalStateException("TableId not known in RFileScanner");
+}
+
+@Override
+public  T instantiate(String className, Class base)
+throws ReflectiveOperationException, IOException {
+  return 
RFileScanner.class.getClassLoader().loadClass(className).asSubclass(base)
+  .getDeclaredConstructor().newInstance();
+}
+
+@Override
+public  T instantiate(TableId tableId, String className, Class 
base)
+throws ReflectiveOperationException, IOException {
+  return instantiate(className, base);
+}
+
+@Override
+public Configuration getConfiguration() {
+  return new ConfigurationImpl(new 
ConfigurationCopy(DefaultConfiguration.getInstance()));
+}
+
+@Override
+public Configuration getConfiguration(TableId tableId) {
+  return new ConfigurationImpl(new 
ConfigurationCopy(opts.tableConfig));
+}
+
+  };
+
+  ClientIteratorEnvironment.Builder iterEnvBuilder =
+  new RFileScannerIteratorEnvironmentBuilder().withEnvironment(senv)
+  .withAuthorizations(opts.auths).withScope(IteratorScope.scan);
+  if (getSamplerConfiguration() != null) {
+iterEnvBuilder.withSamplerConfiguration(getSamplerConfiguration());
+  }
+  IteratorEnvironment iterEnv = iterEnvBuilder.build();

Review Comment:
   Fixed in bf294a4



##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -292,15 +280,54 @@ public Iterator> iterator() {
 EMPTY_BYTES, tableConf);
   }
 
+  ClientServiceEnvironmentImpl senv = new 
ClientServiceEnvironmentImpl(null) {
+
+@Override
+public String getTableName(TableId tableId) throws 
TableNotFoundException {
+  throw new IllegalStateException("TableId not known in RFileScanner");

Review Comment:
   Fixed in bf294a4



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2054540324


##
core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java:
##
@@ -295,9 +230,10 @@ public Iterator> iterator() {
 
 SortedKeyValueIterator skvi;
 try {
-  IteratorEnvironment iterEnv = new ClientSideIteratorEnvironment(
-  getSamplerConfiguration() != null, 
getIteratorSamplerConfigurationInternal());
-
+  IteratorEnvironment iterEnv = new ClientIteratorEnvironment.Builder()
+  .withClient(context.get()).withAuthorizations(getAuthorizations())
+  .withScope(IteratorScope.scan).withTableId(tableId.get())
+  
.withSamplerConfiguration(getIteratorSamplerConfigurationInternal()).build();

Review Comment:
   Fixed in 3cec5f7



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2054539912


##
core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java:
##
@@ -345,9 +245,13 @@ private SortedKeyValueIterator 
createIterator(KeyExtent extent,
 
 MultiIterator multiIter = new MultiIterator(readers, extent);
 
-OfflineIteratorEnvironment iterEnv =
-new OfflineIteratorEnvironment(context, tableId, authorizations, 
tableCC, false,
-samplerConfImpl == null ? null : 
samplerConfImpl.toSamplerConfiguration());
+ClientIteratorEnvironment.Builder iterEnvBuilder =
+new 
ClientIteratorEnvironment.Builder().withAuthorizations(authorizations)
+
.withScope(IteratorScope.scan).withTableId(tableId).withClient(context);
+if (scannerSamplerConfig != null) {
+  iterEnvBuilder.withSamplerConfiguration(scannerSamplerConfig);
+}
+IteratorEnvironment iterEnv = iterEnvBuilder.build();

Review Comment:
   Fixed in 3cec5f7



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2054539541


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.of(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  private static final UnsupportedOperationException UOE =
+  new UnsupportedOperationException("Feature not supported");
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+  }
+
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope.isPresent() ? 
Optional.of(copy.scope.orElseThrow()) : Optional.empty();
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths.isPresent() ? 
Optional.of(copy.auths.orElseThrow()) : Optional.empty();
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId =
+copy.tableId.isPresent() ? Optional.of(copy.tableId.orElseThrow()) : 
Optional.empty();

Review Comment:
   Fixed in 3cec5f7



##
core/src/main/java/org/apache/accumulo/core/iterators/Clie

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2054539180


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.of(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  private static final UnsupportedOperationException UOE =
+  new UnsupportedOperationException("Feature not supported");
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+  }
+
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope.isPresent() ? 
Optional.of(copy.scope.orElseThrow()) : Optional.empty();
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths.isPresent() ? 
Optional.of(copy.auths.orElseThrow()) : Optional.empty();
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId =
+copy.tableId.isPresent() ? Optional.of(copy.tableId.orElseThrow()) : 
Optional.empty();
+this.samplerConfig = copy.samplerConfig.isPresent()
+? Optional.of(copy.samplerConfig.orElseThro

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2054395610


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.of(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  private static final UnsupportedOperationException UOE =
+  new UnsupportedOperationException("Feature not supported");
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+  }
+
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope.isPresent() ? 
Optional.of(copy.scope.orElseThrow()) : Optional.empty();
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths.isPresent() ? 
Optional.of(copy.auths.orElseThrow()) : Optional.empty();
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId =
+copy.tableId.isPresent() ? Optional.of(copy.tableId.orElseThrow()) : 
Optional.empty();
+this.samplerConfig = copy.samplerConfig.isPresent()
+? Optional.of(copy.samplerConfig.orElseThro

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2053120278


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.of(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  private static final UnsupportedOperationException UOE =
+  new UnsupportedOperationException("Feature not supported");
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+  }
+
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope.isPresent() ? 
Optional.of(copy.scope.orElseThrow()) : Optional.empty();
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths.isPresent() ? 
Optional.of(copy.auths.orElseThrow()) : Optional.empty();
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId =
+copy.tableId.isPresent() ? Optional.of(copy.tableId.orElseThrow()) : 
Optional.empty();
+this.samplerConfig = copy.samplerConfig.isPresent()
+? Optional.of(copy.samplerConfig.orElse

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2053140237


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.of(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  private static final UnsupportedOperationException UOE =
+  new UnsupportedOperationException("Feature not supported");
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+  }
+
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope.isPresent() ? 
Optional.of(copy.scope.orElseThrow()) : Optional.empty();
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths.isPresent() ? 
Optional.of(copy.auths.orElseThrow()) : Optional.empty();
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId =
+copy.tableId.isPresent() ? Optional.of(copy.tableId.orElseThrow()) : 
Optional.empty();
+this.samplerConfig = copy.samplerConfig.isPresent()
+? Optional.of(copy.samplerConfig.orElse

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

keith-turner commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2053140237


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.of(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}
+
+public ClientIteratorEnvironment build() {
+  return new ClientIteratorEnvironment(this);
+}
+
+  }
+
+  private static final UnsupportedOperationException UOE =
+  new UnsupportedOperationException("Feature not supported");
+
+  public static final IteratorEnvironment DEFAULT = new Builder().build();
+
+  private final Optional scope;
+  private final boolean isFullMajorCompaction;
+  private final Optional auths;
+  private final boolean isUserCompaction;
+  private final Optional tableId;
+  private final Optional samplerConfig;
+  private final Optional env;
+
+  private ClientIteratorEnvironment(Builder builder) {
+this.scope = builder.scope;
+this.isFullMajorCompaction = builder.isFullMajorCompaction;
+this.auths = builder.auths;
+this.isUserCompaction = builder.isUserCompaction;
+this.tableId = builder.tableId;
+this.samplerConfig = builder.samplerConfig;
+this.env = builder.env;
+  }
+
+  private ClientIteratorEnvironment(ClientIteratorEnvironment copy) {
+this.scope = copy.scope.isPresent() ? 
Optional.of(copy.scope.orElseThrow()) : Optional.empty();
+this.isFullMajorCompaction = copy.isFullMajorCompaction;
+this.auths = copy.auths.isPresent() ? 
Optional.of(copy.auths.orElseThrow()) : Optional.empty();
+this.isUserCompaction = copy.isUserCompaction;
+this.tableId =
+copy.tableId.isPresent() ? Optional.of(copy.tableId.orElseThrow()) : 
Optional.empty();
+this.samplerConfig = copy.samplerConfig.isPresent()
+? Optional.of(copy.samplerConfig.orElse

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2052920199


##
core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java:
##
@@ -345,9 +245,13 @@ private SortedKeyValueIterator 
createIterator(KeyExtent extent,
 
 MultiIterator multiIter = new MultiIterator(readers, extent);
 
-OfflineIteratorEnvironment iterEnv =
-new OfflineIteratorEnvironment(context, tableId, authorizations, 
tableCC, false,
-samplerConfImpl == null ? null : 
samplerConfImpl.toSamplerConfiguration());
+ClientIteratorEnvironment.Builder iterEnvBuilder =
+new 
ClientIteratorEnvironment.Builder().withAuthorizations(authorizations)
+
.withScope(IteratorScope.scan).withTableId(tableId).withClient(context);
+if (scannerSamplerConfig != null) {
+  iterEnvBuilder.withSamplerConfiguration(scannerSamplerConfig);
+}
+IteratorEnvironment iterEnv = iterEnvBuilder.build();

Review Comment:
   Fixed the `samplerConfImpl` issue in c14c35f



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2052918381


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -292,15 +280,54 @@ public Iterator> iterator() {
 EMPTY_BYTES, tableConf);
   }
 
+  ClientServiceEnvironmentImpl senv = new 
ClientServiceEnvironmentImpl(null) {

Review Comment:
   Moved this to a static inner class in c14c35f



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2052918001


##
core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java:
##
@@ -295,9 +230,10 @@ public Iterator> iterator() {
 
 SortedKeyValueIterator skvi;
 try {
-  IteratorEnvironment iterEnv = new ClientSideIteratorEnvironment(
-  getSamplerConfiguration() != null, 
getIteratorSamplerConfigurationInternal());
-
+  IteratorEnvironment iterEnv = new ClientIteratorEnvironment.Builder()
+  .withClient(context.get()).withAuthorizations(getAuthorizations())
+  .withScope(IteratorScope.scan).withTableId(tableId.get())
+  
.withSamplerConfiguration(getIteratorSamplerConfigurationInternal()).build();

Review Comment:
   I think I fixed this in c14c35f



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2052898509


##
core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java:
##
@@ -345,9 +245,13 @@ private SortedKeyValueIterator 
createIterator(KeyExtent extent,
 
 MultiIterator multiIter = new MultiIterator(readers, extent);
 
-OfflineIteratorEnvironment iterEnv =
-new OfflineIteratorEnvironment(context, tableId, authorizations, 
tableCC, false,
-samplerConfImpl == null ? null : 
samplerConfImpl.toSamplerConfiguration());
+ClientIteratorEnvironment.Builder iterEnvBuilder =
+new 
ClientIteratorEnvironment.Builder().withAuthorizations(authorizations)
+
.withScope(IteratorScope.scan).withTableId(tableId).withClient(context);
+if (scannerSamplerConfig != null) {
+  iterEnvBuilder.withSamplerConfiguration(scannerSamplerConfig);
+}
+IteratorEnvironment iterEnv = iterEnvBuilder.build();

Review Comment:
   Regarding the missing `tableCC`, by setting `withClient(ClientContext)`, it 
will create a `ClientServiceEnvironmentImpl` for the calls to ` getServiceEnv` 
and `getPluginEnv` and the table configuration can be retrieved from that 
object.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2052895109


##
core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java:
##
@@ -345,9 +245,13 @@ private SortedKeyValueIterator 
createIterator(KeyExtent extent,
 
 MultiIterator multiIter = new MultiIterator(readers, extent);
 
-OfflineIteratorEnvironment iterEnv =
-new OfflineIteratorEnvironment(context, tableId, authorizations, 
tableCC, false,
-samplerConfImpl == null ? null : 
samplerConfImpl.toSamplerConfiguration());
+ClientIteratorEnvironment.Builder iterEnvBuilder =
+new 
ClientIteratorEnvironment.Builder().withAuthorizations(authorizations)
+
.withScope(IteratorScope.scan).withTableId(tableId).withClient(context);
+if (scannerSamplerConfig != null) {
+  iterEnvBuilder.withSamplerConfiguration(scannerSamplerConfig);
+}
+IteratorEnvironment iterEnv = iterEnvBuilder.build();

Review Comment:
   The old code set useSampling to `false`, but went to the trouble of creating 
the `sampleConfiguration`. I'm wondering if setting `useSampling` to `false` 
was a bug in the old version.
   
   
https://github.com/apache/accumulo/blob/3912b36d0b0ca103b7068d7c7575c99d32a11a97/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java#L324-L331



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2052870189


##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.clientImpl.ClientServiceEnvironmentImpl;
+import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.data.Value;
+import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope;
+import org.apache.accumulo.core.iteratorsImpl.system.MapFileIterator;
+import org.apache.accumulo.core.security.Authorizations;
+import org.apache.accumulo.core.spi.common.ServiceEnvironment;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+
+public class ClientIteratorEnvironment implements IteratorEnvironment {
+
+  public static class Builder {
+
+private Optional scope = Optional.empty();
+private boolean isFullMajorCompaction = false;
+private Optional auths = Optional.empty();
+private boolean isUserCompaction = false;
+private Optional tableId = Optional.empty();
+private Optional samplerConfig = Optional.empty();
+protected Optional env = Optional.empty();
+
+public Builder withScope(IteratorScope scope) {
+  this.scope = Optional.of(scope);
+  return this;
+}
+
+public Builder isFullMajorCompaction() {
+  this.isFullMajorCompaction = true;
+  return this;
+}
+
+public Builder withAuthorizations(Authorizations auths) {
+  this.auths = Optional.of(auths);
+  return this;
+}
+
+public Builder isUserCompaction() {
+  this.isUserCompaction = true;
+  return this;
+}
+
+public Builder withTableId(TableId tableId) {
+  this.tableId = Optional.of(tableId);
+  return this;
+}
+
+public Builder withSamplerConfiguration(SamplerConfiguration sc) {
+  this.samplerConfig = Optional.of(sc);
+  return this;
+}
+
+public Builder withClient(AccumuloClient client) {
+  this.env = Optional.of(new ClientServiceEnvironmentImpl((ClientContext) 
client));
+  return this;
+}

Review Comment:
   I had that earlier, and it fails the build. `o.a.a.core.iterators` is in the 
public API, so this method needs to use AccumuloClient vs ClientContext as the 
API parameter.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

kevinrr888 commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2052762928


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -292,15 +280,54 @@ public Iterator> iterator() {
 EMPTY_BYTES, tableConf);
   }
 
+  ClientServiceEnvironmentImpl senv = new 
ClientServiceEnvironmentImpl(null) {
+
+@Override
+public String getTableName(TableId tableId) throws 
TableNotFoundException {
+  throw new IllegalStateException("TableId not known in RFileScanner");
+}
+
+@Override
+public  T instantiate(String className, Class base)
+throws ReflectiveOperationException, IOException {
+  return 
RFileScanner.class.getClassLoader().loadClass(className).asSubclass(base)
+  .getDeclaredConstructor().newInstance();
+}
+
+@Override
+public  T instantiate(TableId tableId, String className, Class 
base)
+throws ReflectiveOperationException, IOException {
+  return instantiate(className, base);
+}
+
+@Override
+public Configuration getConfiguration() {
+  return new ConfigurationImpl(new 
ConfigurationCopy(DefaultConfiguration.getInstance()));
+}
+
+@Override
+public Configuration getConfiguration(TableId tableId) {
+  return new ConfigurationImpl(new 
ConfigurationCopy(opts.tableConfig));
+}
+
+  };
+
+  ClientIteratorEnvironment.Builder iterEnvBuilder =
+  new RFileScannerIteratorEnvironmentBuilder().withEnvironment(senv)
+  .withAuthorizations(opts.auths).withScope(IteratorScope.scan);
+  if (getSamplerConfiguration() != null) {
+iterEnvBuilder.withSamplerConfiguration(getSamplerConfiguration());
+  }
+  IteratorEnvironment iterEnv = iterEnvBuilder.build();

Review Comment:
   I believe this needs a `withTableId`. While a table id doesn't make sense in 
this case, the use case would be:
   `env.getPluginEnv().getConfiguration(env.getTableId())`
   See discussions: https://github.com/apache/accumulo/issues/4810 and 
https://github.com/apache/accumulo/pull/4816/files#r1725159871 and 
https://github.com/apache/accumulo/pull/4816#issuecomment-234716 (2nd 
section)



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

kevinrr888 commented on code in PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#discussion_r2052732655


##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -292,15 +280,54 @@ public Iterator> iterator() {
 EMPTY_BYTES, tableConf);
   }
 
+  ClientServiceEnvironmentImpl senv = new 
ClientServiceEnvironmentImpl(null) {

Review Comment:
   Might be nicer to avoid anon class



##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -292,15 +280,54 @@ public Iterator> iterator() {
 EMPTY_BYTES, tableConf);
   }
 
+  ClientServiceEnvironmentImpl senv = new 
ClientServiceEnvironmentImpl(null) {
+
+@Override
+public String getTableName(TableId tableId) throws 
TableNotFoundException {
+  throw new IllegalStateException("TableId not known in RFileScanner");

Review Comment:
   ```
   private static final String errorMsg =
 "This scanner is unrelated to any table or accumulo instance;"
 + " it operates directly on files. Therefore, it can not support 
this operation.";
   ```
   Is the error message used in #4816, could keep this or something similar 
since it's more descriptive



##
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java:
##
@@ -292,15 +280,54 @@ public Iterator> iterator() {
 EMPTY_BYTES, tableConf);
   }
 
+  ClientServiceEnvironmentImpl senv = new 
ClientServiceEnvironmentImpl(null) {
+
+@Override
+public String getTableName(TableId tableId) throws 
TableNotFoundException {
+  throw new IllegalStateException("TableId not known in RFileScanner");
+}
+
+@Override
+public  T instantiate(String className, Class base)
+throws ReflectiveOperationException, IOException {
+  return 
RFileScanner.class.getClassLoader().loadClass(className).asSubclass(base)
+  .getDeclaredConstructor().newInstance();
+}
+
+@Override
+public  T instantiate(TableId tableId, String className, Class 
base)
+throws ReflectiveOperationException, IOException {
+  return instantiate(className, base);
+}
+
+@Override
+public Configuration getConfiguration() {
+  return new ConfigurationImpl(new 
ConfigurationCopy(DefaultConfiguration.getInstance()));
+}

Review Comment:
   In #4816, had this method throwing an exception. Don't remember the 
specifics, but should make sure this functions as we expect (should it throw an 
exception, or return something)



##
core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java:
##
@@ -345,9 +245,13 @@ private SortedKeyValueIterator 
createIterator(KeyExtent extent,
 
 MultiIterator multiIter = new MultiIterator(readers, extent);
 
-OfflineIteratorEnvironment iterEnv =
-new OfflineIteratorEnvironment(context, tableId, authorizations, 
tableCC, false,
-samplerConfImpl == null ? null : 
samplerConfImpl.toSamplerConfiguration());
+ClientIteratorEnvironment.Builder iterEnvBuilder =
+new 
ClientIteratorEnvironment.Builder().withAuthorizations(authorizations)
+
.withScope(IteratorScope.scan).withTableId(tableId).withClient(context);
+if (scannerSamplerConfig != null) {
+  iterEnvBuilder.withSamplerConfiguration(scannerSamplerConfig);
+}
+IteratorEnvironment iterEnv = iterEnvBuilder.build();

Review Comment:
   It seems usage of `tableCC` and `samplerConfImpl` were dropped. Was this 
intentional?



##
core/src/main/java/org/apache/accumulo/core/iterators/ClientIteratorEnvironment.java:
##
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.iterators;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.client.PluginEnvironment;
+import org.apache.accumulo.core.client.SampleNotPresentException;
+import org.apache.accumulo.core.client.sample.SamplerConfiguration;
+import org

Re: [PR] Modified IteratorEnvironment to no longer throw UOE [accumulo]

[via GitHub]

dlmarion commented on PR #5490:
URL: https://github.com/apache/accumulo/pull/5490#issuecomment-2816219509

   The replacement of the `IteratorEnvironment` implementations in 
`ClientSideIteratorScanner`, `RFileScanner`, `OfflineIterator` with 
`ClientIteratorEnvironment` need to be scrutinized. I know that in one case one 
of the implementations handled the `registerSideChannel` method, but I could 
not find it being used anywhere so I just deleted it.


-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org