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 2020/08/18 04:14:57 UTC
[GitHub] [accumulo] ctubbsii opened a new pull request #1688: Fix issues in prep for 1.10 release
ctubbsii opened a new pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688
Fix quality issues in prep for 1.10 release
* Converge the Hadoop 2 and Hadoop 3 assemblies, so only one is
necessary (tested both on Hadoop 2.6.5, 3.0.3, and 3.3.0 using
fluo-uno)
* Bumped plugin versions and added timestamp flag for increased build
reproducibility; timestamp should automatically update during release
process; verified the built jars differ only in the contents of the
generated META-INF/DEPENDENCIES file when building with the Hadoop2
vs. the Hadoop3 profiles, and since this file doesn't matter, they are
now provably equivalent builds
* Fixed some Hadoop profile issues, and updated Hadoop versions to
latest patch version in the minor release lines we had previously been
building with as our minimum supported versions
* Bump Guava, and related code to work with both Hadoop 2 and 3;
suppress warnings or migrate code to avoid code paths that won't work
across Guava versions, for fewer class path surprises related to
Guava
* Update asciidoc plugin, and fix various doc build issues warned about
by the plugin
Bump snapshot versions to 1.10.0-SNAPSHOT
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-675241586
Ugh! Class path issues with VFS and Hadoop and Guava versions... causing build failures. I'll fix it tomorrow. @EdColeman This patch isn't ready to merge, but it did resolve all the class path issues we discussed in Slack regarding trying to run things in Uno.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-675722581
That was similar to my test case: run shell, create table, insert entry, flush, compact, delete table, grep for `ERR` in logs, and inspect monitor, repeat for Hadoop 2.6.5, 3.0.3, and 3.3.0. I especially liked that the same binary tarball worked for Hadoop 2 and Hadoop 3... I even verified the class files in the jar built identically with both profiles.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii edited a comment on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii edited a comment on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-675722581
That was similar to my test case: run shell, create table, insert entry, flush, compact, delete table, grep for `ERR` in logs, and inspect monitor, repeat for Hadoop 2.6.5, 3.0.3, and 3.3.0. I especially liked that the same binary tarball worked for Hadoop 2 and Hadoop 3... I even verified the class files in the jar built identically with both profiles.
I'll run through this test case again tomorrow to double-check, before merging.
I might also run the full ITs overnight.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a change in pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#discussion_r473312309
##########
File path: pom.xml
##########
@@ -136,10 +136,12 @@
<maven.compiler.release>8</maven.compiler.release>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
- <maven.plugin-version>3.0.5</maven.plugin-version>
+ <maven.plugin-version>3.5.0</maven.plugin-version>
<!-- surefire/failsafe plugin option -->
<maven.test.redirectTestOutputToFile>true</maven.test.redirectTestOutputToFile>
<powermock.version>2.0.2</powermock.version>
+ <!-- timestamp for reproducible outputs, updated on release by the release plugin -->
+ <project.build.outputTimestamp>2020-08-18T00:00:00Z</project.build.outputTimestamp>
Review comment:
Is this an artifact of release testing?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#discussion_r474183577
##########
File path: server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java
##########
@@ -104,28 +105,27 @@ public void run() {
Span findWalsSpan = Trace.start("findReferencedWals");
HashSet<String> closed = null;
try {
- sw.start();
+ startTime = System.nanoTime();
closed = getClosedLogs(conn);
+ duration = Duration.ofNanos(System.nanoTime() - startTime);
} finally {
- sw.stop();
findWalsSpan.stop();
}
- log.info("Found " + closed.size() + " WALs referenced in metadata in " + sw.toString());
- sw.reset();
+ log.info("Found " + closed.size() + " WALs referenced in metadata in " + duration);
Review comment:
Probbaily just a nit, but does changing the output format matter? Duration will print out like: PT0.021850552S , Stopwatch is more "user" friendly with "333.2 ms"
Also, maybe unrelated by anything other than name, but there is a StopWatch class in org.apache.accumulo.core.util - it seems to be referenced only in org.apache.accumulo.server.client.BulkImporter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a change in pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#discussion_r474183577
##########
File path: server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java
##########
@@ -104,28 +105,27 @@ public void run() {
Span findWalsSpan = Trace.start("findReferencedWals");
HashSet<String> closed = null;
try {
- sw.start();
+ startTime = System.nanoTime();
closed = getClosedLogs(conn);
+ duration = Duration.ofNanos(System.nanoTime() - startTime);
} finally {
- sw.stop();
findWalsSpan.stop();
}
- log.info("Found " + closed.size() + " WALs referenced in metadata in " + sw.toString());
- sw.reset();
+ log.info("Found " + closed.size() + " WALs referenced in metadata in " + duration);
Review comment:
Probably just a nit, but does changing the output format matter? Duration will print out like: PT0.021850552S , Stopwatch is more "user" friendly with "333.2 ms"
Also, maybe unrelated by anything other than name, but there is a StopWatch class in org.apache.accumulo.core.util - it seems to be referenced only in org.apache.accumulo.server.client.BulkImporter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#discussion_r474187125
##########
File path: server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java
##########
@@ -104,28 +105,27 @@ public void run() {
Span findWalsSpan = Trace.start("findReferencedWals");
HashSet<String> closed = null;
try {
- sw.start();
+ startTime = System.nanoTime();
closed = getClosedLogs(conn);
+ duration = Duration.ofNanos(System.nanoTime() - startTime);
} finally {
- sw.stop();
findWalsSpan.stop();
}
- log.info("Found " + closed.size() + " WALs referenced in metadata in " + sw.toString());
- sw.reset();
+ log.info("Found " + closed.size() + " WALs referenced in metadata in " + duration);
Review comment:
I tried really hard to preserve the format, without writing or copying code, and still supporting the various Guava versions, which had breaking changes in Stopwatch, and finally gave up. I don't think the format matters as much as the information... and I'm not sure the information even matters that much.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#discussion_r473370177
##########
File path: pom.xml
##########
@@ -136,10 +136,12 @@
<maven.compiler.release>8</maven.compiler.release>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
- <maven.plugin-version>3.0.5</maven.plugin-version>
+ <maven.plugin-version>3.5.0</maven.plugin-version>
<!-- surefire/failsafe plugin option -->
<maven.test.redirectTestOutputToFile>true</maven.test.redirectTestOutputToFile>
<powermock.version>2.0.2</powermock.version>
+ <!-- timestamp for reproducible outputs, updated on release by the release plugin -->
+ <project.build.outputTimestamp>2020-08-18T00:00:00Z</project.build.outputTimestamp>
Review comment:
This property is for reproducible builds. The timestamp is used by several plugins where timestamps would have caused the plugin to produce different binaries. See https://maven.apache.org/guides/mini/guide-reproducible-builds.html
The property itself is supposed to be updated in the POM by the release plugin when it updates the version number during the release process. I have not tested, but will look for that during RC testing when it comes to that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii merged pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-675680354
I was wrong. We don't need to bundle different versions... we just need to downgrade commons-io back to 2.6, because that is causing problems with vfs2/guava/hadoop/hadoop-minicluster. See my comment at https://github.com/apache/accumulo/pull/1687#issuecomment-675669983
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-675717519
I was able to take this branch and run an instance with uno and did not see issues. Used hadoop 3.0.3. For commands I ran the shell, created a table and set and deleted a property. Not extensive, but if not resolved, clearly a lot closer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-675596158
Okay, so I think we have to bundle a different version of the guava and commons-vfs2 jars, depending on the build profile. It's a simple thing to do, but I need to finish ensuring tests pass on both.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-677866175
Okay, I am able to verify full ITs with this change for 3.0.3 and 2.6.5, and after much testing with fluo-uno, have verified basic functionality (create table, insert data, flush, compact, scan, delete table), with Hadoop versions: 2.6.5, 2.7.7, 2.8.5, 2.9.2, 2.10.0, 3.0.3, 3.1.3, 3.2.1, and 3.3.0.
The only quirk was that I had to replace Guava 14.0.1 with 27.0-jre in order for Hadoop 3.1.3, 3.2.1, and 3.3.0 to work without errors.
I'll be glad when we stop supporting our 1.x branch with both Hadoop 2 and Hadoop 3 support. The matrix of possible versions for downstream users to think about is getting pretty insanely large.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman edited a comment on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
EdColeman edited a comment on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-675717519
I was able to take this branch and run an instance with uno and did not see issues. Used hadoop 3.0.3. For commands I ran the shell, created a table and set and deleted a property. In addition, the normal background processes (compactions) were running without error.
Not extensive, but if not resolved, clearly a lot closer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#issuecomment-676605419
Full ITs passed with Hadoop 3.0.3 and 2.6.5, but I'm still getting class path issues with Hadoop 3.3 and 3.2. I need to check 2.9 and 3.1. For 3.3 and 3.2, it may be a simple matter of building Accumulo with a newer VFS and Guava. I'm not sure. There are getting to be *far* too many combinations to easily test. I'm thinking about settling for compatibility with 2.x and 3.0, and being done with 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a change in pull request #1688: Fix issues in prep for 1.10 release
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1688:
URL: https://github.com/apache/accumulo/pull/1688#discussion_r473396996
##########
File path: pom.xml
##########
@@ -136,10 +136,12 @@
<maven.compiler.release>8</maven.compiler.release>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
- <maven.plugin-version>3.0.5</maven.plugin-version>
+ <maven.plugin-version>3.5.0</maven.plugin-version>
<!-- surefire/failsafe plugin option -->
<maven.test.redirectTestOutputToFile>true</maven.test.redirectTestOutputToFile>
<powermock.version>2.0.2</powermock.version>
+ <!-- timestamp for reproducible outputs, updated on release by the release plugin -->
+ <project.build.outputTimestamp>2020-08-18T00:00:00Z</project.build.outputTimestamp>
Review comment:
Ok. I thought maybe you were testing the maven release process and this was accidentally introduced.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org