You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/07/25 17:36:08 UTC
[GitHub] [accumulo-testing] DomGarguilo opened a new pull request, #212: Miscellaneous improvements to continuous code
DomGarguilo opened a new pull request, #212:
URL: https://github.com/apache/accumulo-testing/pull/212
These changes include:
* Add a `Random` member variable to `TestEnv` to be used where possible instead of constructing new ones when needed.
* Add more `AutoClosable`s to try-with-resources blocks
* Simplify some logic using streams
* Improve some log messages
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] milleruntime commented on a diff in pull request #212: Miscellaneous improvements to continuous code
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #212:
URL: https://github.com/apache/accumulo-testing/pull/212#discussion_r936806526
##########
src/main/java/org/apache/accumulo/testing/continuous/ContinuousBatchWalker.java:
##########
@@ -39,34 +40,33 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Sets;
+
public class ContinuousBatchWalker {
private static final Logger log = LoggerFactory.getLogger(ContinuousBatchWalker.class);
public static void main(String[] args) throws Exception {
try (ContinuousEnv env = new ContinuousEnv(args)) {
- Authorizations auths = env.getRandomAuthorizations();
- AccumuloClient client = env.getAccumuloClient();
- Scanner scanner = ContinuousUtil.createScanner(client, env.getAccumuloTableName(), auths);
- int scanBatchSize = Integer.parseInt(env.getTestProperty(TestProps.CI_BW_BATCH_SIZE));
- scanner.setBatchSize(scanBatchSize);
-
- Random r = new Random();
-
- while (true) {
- BatchScanner bs = client.createBatchScanner(env.getAccumuloTableName(), auths);
- Set<Text> batch = getBatch(scanner, env.getRowMin(), env.getRowMax(), scanBatchSize, r);
- List<Range> ranges = new ArrayList<>(batch.size());
+ Authorizations auths = env.getRandomAuthorizations();
- for (Text row : batch) {
- ranges.add(new Range(row));
+ try (AccumuloClient client = env.getAccumuloClient();
Review Comment:
Client doesn't need to be in the try-with-resources.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] milleruntime commented on a diff in pull request #212: Miscellaneous improvements to continuous code
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #212:
URL: https://github.com/apache/accumulo-testing/pull/212#discussion_r936807043
##########
src/main/java/org/apache/accumulo/testing/continuous/ContinuousScanner.java:
##########
@@ -40,58 +34,51 @@ public class ContinuousScanner {
public static void main(String[] args) throws Exception {
- try (ContinuousEnv env = new ContinuousEnv(args)) {
+ try (ContinuousEnv env = new ContinuousEnv(args);
+ AccumuloClient client = env.getAccumuloClient()) {
Review Comment:
Same here.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] DomGarguilo commented on a diff in pull request #212: Miscellaneous improvements to continuous code
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #212:
URL: https://github.com/apache/accumulo-testing/pull/212#discussion_r930008733
##########
src/main/java/org/apache/accumulo/testing/continuous/ContinuousBatchWalker.java:
##########
@@ -39,34 +40,33 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Sets;
+
public class ContinuousBatchWalker {
private static final Logger log = LoggerFactory.getLogger(ContinuousBatchWalker.class);
public static void main(String[] args) throws Exception {
try (ContinuousEnv env = new ContinuousEnv(args)) {
- Authorizations auths = env.getRandomAuthorizations();
- AccumuloClient client = env.getAccumuloClient();
- Scanner scanner = ContinuousUtil.createScanner(client, env.getAccumuloTableName(), auths);
- int scanBatchSize = Integer.parseInt(env.getTestProperty(TestProps.CI_BW_BATCH_SIZE));
- scanner.setBatchSize(scanBatchSize);
-
- Random r = new Random();
-
- while (true) {
- BatchScanner bs = client.createBatchScanner(env.getAccumuloTableName(), auths);
- Set<Text> batch = getBatch(scanner, env.getRowMin(), env.getRowMax(), scanBatchSize, r);
- List<Range> ranges = new ArrayList<>(batch.size());
+ Authorizations auths = env.getRandomAuthorizations();
- for (Text row : batch) {
- ranges.add(new Range(row));
+ try (AccumuloClient client = env.getAccumuloClient();
Review Comment:
Looks like it will be reused as long as its not null, but `env` is probably cleaned up before it can be reused anyways
https://github.com/apache/accumulo-testing/blob/135d37dcfe8927054778ef4a5702f731e4fb21fd/src/main/java/org/apache/accumulo/testing/TestEnv.java#L172-L177
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] milleruntime commented on a diff in pull request #212: Miscellaneous improvements to continuous code
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #212:
URL: https://github.com/apache/accumulo-testing/pull/212#discussion_r936804897
##########
src/main/java/org/apache/accumulo/testing/continuous/ContinuousBatchWalker.java:
##########
@@ -39,34 +40,33 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Sets;
+
public class ContinuousBatchWalker {
private static final Logger log = LoggerFactory.getLogger(ContinuousBatchWalker.class);
public static void main(String[] args) throws Exception {
try (ContinuousEnv env = new ContinuousEnv(args)) {
- Authorizations auths = env.getRandomAuthorizations();
- AccumuloClient client = env.getAccumuloClient();
- Scanner scanner = ContinuousUtil.createScanner(client, env.getAccumuloTableName(), auths);
- int scanBatchSize = Integer.parseInt(env.getTestProperty(TestProps.CI_BW_BATCH_SIZE));
- scanner.setBatchSize(scanBatchSize);
-
- Random r = new Random();
-
- while (true) {
- BatchScanner bs = client.createBatchScanner(env.getAccumuloTableName(), auths);
- Set<Text> batch = getBatch(scanner, env.getRowMin(), env.getRowMax(), scanBatchSize, r);
- List<Range> ranges = new ArrayList<>(batch.size());
+ Authorizations auths = env.getRandomAuthorizations();
- for (Text row : batch) {
- ranges.add(new Range(row));
+ try (AccumuloClient client = env.getAccumuloClient();
Review Comment:
OK its probably not a big deal then but I'd remove the try-with-resources around the client since its redundant.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] milleruntime commented on a diff in pull request #212: Miscellaneous improvements to continuous code
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #212:
URL: https://github.com/apache/accumulo-testing/pull/212#discussion_r929903410
##########
src/main/java/org/apache/accumulo/testing/continuous/ContinuousBatchWalker.java:
##########
@@ -39,34 +40,33 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Sets;
+
public class ContinuousBatchWalker {
private static final Logger log = LoggerFactory.getLogger(ContinuousBatchWalker.class);
public static void main(String[] args) throws Exception {
try (ContinuousEnv env = new ContinuousEnv(args)) {
- Authorizations auths = env.getRandomAuthorizations();
- AccumuloClient client = env.getAccumuloClient();
- Scanner scanner = ContinuousUtil.createScanner(client, env.getAccumuloTableName(), auths);
- int scanBatchSize = Integer.parseInt(env.getTestProperty(TestProps.CI_BW_BATCH_SIZE));
- scanner.setBatchSize(scanBatchSize);
-
- Random r = new Random();
-
- while (true) {
- BatchScanner bs = client.createBatchScanner(env.getAccumuloTableName(), auths);
- Set<Text> batch = getBatch(scanner, env.getRowMin(), env.getRowMax(), scanBatchSize, r);
- List<Range> ranges = new ArrayList<>(batch.size());
+ Authorizations auths = env.getRandomAuthorizations();
- for (Text row : batch) {
- ranges.add(new Range(row));
+ try (AccumuloClient client = env.getAccumuloClient();
Review Comment:
Does the client get reused? I am wondering since it gets pulled from the `env`. I don't think it does, its just a convenience method...
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] DomGarguilo merged pull request #212: Miscellaneous improvements to continuous code
Posted by GitBox <gi...@apache.org>.
DomGarguilo merged PR #212:
URL: https://github.com/apache/accumulo-testing/pull/212
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-testing] DomGarguilo commented on a diff in pull request #212: Miscellaneous improvements to continuous code
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #212:
URL: https://github.com/apache/accumulo-testing/pull/212#discussion_r937002977
##########
src/main/java/org/apache/accumulo/testing/continuous/ContinuousBatchWalker.java:
##########
@@ -39,34 +40,33 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Sets;
+
public class ContinuousBatchWalker {
private static final Logger log = LoggerFactory.getLogger(ContinuousBatchWalker.class);
public static void main(String[] args) throws Exception {
try (ContinuousEnv env = new ContinuousEnv(args)) {
- Authorizations auths = env.getRandomAuthorizations();
- AccumuloClient client = env.getAccumuloClient();
- Scanner scanner = ContinuousUtil.createScanner(client, env.getAccumuloTableName(), auths);
- int scanBatchSize = Integer.parseInt(env.getTestProperty(TestProps.CI_BW_BATCH_SIZE));
- scanner.setBatchSize(scanBatchSize);
-
- Random r = new Random();
-
- while (true) {
- BatchScanner bs = client.createBatchScanner(env.getAccumuloTableName(), auths);
- Set<Text> batch = getBatch(scanner, env.getRowMin(), env.getRowMax(), scanBatchSize, r);
- List<Range> ranges = new ArrayList<>(batch.size());
+ Authorizations auths = env.getRandomAuthorizations();
- for (Text row : batch) {
- ranges.add(new Range(row));
+ try (AccumuloClient client = env.getAccumuloClient();
Review Comment:
Addressed these in [8a658d6](https://github.com/apache/accumulo-testing/pull/212/commits/8a658d64fe381d511292c24145cd46602d5bf66f)
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org