keith-turner commented on PR #3801:
URL: https://github.com/apache/accumulo/pull/3801#issuecomment-1749904934
@dlmarion if you have some time to chat sometime I would like to discuss
some questions I have. Wondering about the following.
Where things run? Maybe instead of having a ta
keith-turner commented on issue #3798:
URL: https://github.com/apache/accumulo/issues/3798#issuecomment-1749885670
@cshannon renaming `getFirstKey()` and `getLastKey()` to `getFirstRow()`
and `getLastRow()` seems like the best option to me if all uses only need the
row. Just need to do t
ctubbsii commented on code in PR #3770:
URL: https://github.com/apache/accumulo/pull/3770#discussion_r1348084591
##
server/monitor/src/main/appended-resources/META-INF/LICENSE:
##
@@ -75,14 +75,23 @@ Files:
* Dual licensed under the MIT and GPL licenses.
* http://ben
ctubbsii opened a new pull request, #3819:
URL: https://github.com/apache/accumulo/pull/3819
* Mitigate CVE-2023-33201 in bouncycastle test dependency by updating to
1.76; it's unlikely we'd have hit this issue, because we only use this for a
test dependency for our Ssl tests, but it is sti
keith-turner commented on code in PR #3817:
URL: https://github.com/apache/accumulo/pull/3817#discussion_r1348023311
##
pom.xml:
##
@@ -981,8 +954,6 @@
org.apache.logging.log4j:log4j-1.2-api:jar:*
org.apache.logging.log4j:log4j-slf4j2-impl:jar:
keith-turner commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749689324
> I'm wondering if we should just close this PR and punt on Java 21 source
compatibility for now.
It feels like this JDK version has improved the static analysis it does, so
keith-turner commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1348010818
##
core/src/main/java/org/apache/accumulo/core/security/Authorizations.java:
##
@@ -43,11 +43,11 @@
public class Authorizations implements Iterable, Serializable
dlmarion commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1348004469
##
core/src/main/java/org/apache/accumulo/core/security/Authorizations.java:
##
@@ -43,11 +43,11 @@
public class Authorizations implements Iterable, Serializable,
A
ctubbsii commented on PR #3817:
URL: https://github.com/apache/accumulo/pull/3817#issuecomment-1749676002
> I believe that we have sufficient coverage.
Okay, that's the part I was concerned about. If we have sufficient coverage,
I'm okay with moving forward on dropping these, so long
ctubbsii commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749673947
> I'm wondering if we should just close this PR and punt on Java 21 source
compatibility for now.
Can just leave it open until we work through the issues. No need to close
it. It
ctubbsii commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1348002382
##
pom.xml:
##
@@ -783,6 +783,7 @@
true
-Xlint:all
+ -Xlint:-this-escape
Review Comment:
I'd rather just fi
ctubbsii commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1347999615
##
core/src/main/java/org/apache/accumulo/core/security/Authorizations.java:
##
@@ -43,11 +43,11 @@
public class Authorizations implements Iterable, Serializable,
A
ctubbsii commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1347999615
##
core/src/main/java/org/apache/accumulo/core/security/Authorizations.java:
##
@@ -43,11 +43,11 @@
public class Authorizations implements Iterable, Serializable,
A
keith-turner commented on PR #3818:
URL: https://github.com/apache/accumulo/pull/3818#issuecomment-1749663924
> We might want to have a test somewhere that test's Scanner/BatchScanner
re-use with concurrent iterators with different settings if we don't have one
already.
That would be
dlmarion commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749663796
I'm wondering if we should just close this PR and punt on Java 21 source
compatibility for now.
--
This is an automated message from the Apache Git Service.
To respond to the message
ctubbsii merged PR #24:
URL: https://github.com/apache/accumulo-access/pull/24
--
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...
keith-turner commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1347993961
##
core/src/main/java/org/apache/accumulo/core/security/Authorizations.java:
##
@@ -43,11 +43,11 @@
public class Authorizations implements Iterable, Serializable
dlmarion commented on PR #3818:
URL: https://github.com/apache/accumulo/pull/3818#issuecomment-1749660079
We might want to have a test somewhere that test's Scanner/BatchScanner
re-use with concurrent iterators with different settings if we don't have one
already.
--
This is an automated
keith-turner commented on PR #3818:
URL: https://github.com/apache/accumulo/pull/3818#issuecomment-1749655385
> We could have retrieved the iterator in the foreground then and passed it
to the background thread making the setConsistencyLevel and iterator calls done
serially in the foregroun
dlmarion commented on PR #3818:
URL: https://github.com/apache/accumulo/pull/3818#issuecomment-1749650721
We could have retrieved the iterator in the foreground then and passed it to
the background thread making the `setConsistencyLevel` and `iterator` calls
done serially in the foreground?
dlmarion merged PR #3816:
URL: https://github.com/apache/accumulo/pull/3816
--
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...@ac
keith-turner commented on PR #3818:
URL: https://github.com/apache/accumulo/pull/3818#issuecomment-1749647822
> what's the race condition? I thought that you could change the settings
on a scanner and call iterator() at any point.
The consistency level was set on the scanner in the f
dlmarion commented on PR #3817:
URL: https://github.com/apache/accumulo/pull/3817#issuecomment-1749645438
> Why do this? Didn't we want this test coverage?
As I mentioned in the JDK 21 PR, I don't think these PowerMock tests are
going to be compatible with Java 21 and I believe that w
dlmarion commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1347967922
##
core/src/main/java/org/apache/accumulo/core/security/Authorizations.java:
##
@@ -43,11 +43,11 @@
public class Authorizations implements Iterable, Serializable,
A
dlmarion commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1347967922
##
core/src/main/java/org/apache/accumulo/core/security/Authorizations.java:
##
@@ -43,11 +43,11 @@
public class Authorizations implements Iterable, Serializable,
A
keith-turner commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1347966000
##
pom.xml:
##
@@ -783,6 +783,7 @@
true
-Xlint:all
+ -Xlint:-this-escape
Review Comment:
Could add supp
keith-turner commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1347967061
##
core/src/main/java/org/apache/accumulo/core/security/Authorizations.java:
##
@@ -43,11 +43,11 @@
public class Authorizations implements Iterable, Serializable
dlmarion commented on PR #3818:
URL: https://github.com/apache/accumulo/pull/3818#issuecomment-1749633279
what's the race condition? I thought that you could change the settings on a
scanner and call `iterator()` at any point.
--
This is an automated message from the Apache Git Service.
keith-turner merged PR #3818:
URL: https://github.com/apache/accumulo/pull/3818
--
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..
ctubbsii commented on code in PR #24:
URL: https://github.com/apache/accumulo-access/pull/24#discussion_r1347946006
##
src/test/java/org/apache/accumulo/access/AccessExpressionBenchmark.java:
##
@@ -117,8 +118,8 @@ List getStringExpressions() {
return allTestExpressionsSt
DomGarguilo commented on code in PR #3817:
URL: https://github.com/apache/accumulo/pull/3817#discussion_r1347941978
##
pom.xml:
##
@@ -981,8 +954,6 @@
org.apache.logging.log4j:log4j-1.2-api:jar:*
org.apache.logging.log4j:log4j-slf4j2-impl:jar:*
ctubbsii commented on code in PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#discussion_r1347937751
##
pom.xml:
##
@@ -783,6 +783,7 @@
true
-Xlint:all
+ -Xlint:-this-escape
Review Comment:
I don't think this
DomGarguilo commented on code in PR #3817:
URL: https://github.com/apache/accumulo/pull/3817#discussion_r1347940076
##
pom.xml:
##
@@ -981,8 +954,6 @@
org.apache.logging.log4j:log4j-1.2-api:jar:*
org.apache.logging.log4j:log4j-slf4j2-impl:jar:*
ctubbsii commented on PR #3817:
URL: https://github.com/apache/accumulo/pull/3817#issuecomment-1749585933
Why do this? Didn't we want this test coverage?
--
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
keith-turner opened a new pull request, #3818:
URL: https://github.com/apache/accumulo/pull/3818
(no comment)
--
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
EdColeman merged PR #3814:
URL: https://github.com/apache/accumulo/pull/3814
--
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...@a
dlmarion commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749387891
Created #3817 to discuss removing the tests and PowerMock.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
U
dlmarion opened a new pull request, #3817:
URL: https://github.com/apache/accumulo/pull/3817
Removed CompactionCoordinatorTest and CompactorTest which were testing the
state of the CompactionCoordinator and Compactor internals. Removing these
tests allowed me to remove PowerMock and JUnit4
dlmarion commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749374379
> [Mockito](https://site.mockito.org/) may also be an alternative.
I looked at their site and did not get a good feel that it would work with
Java 21 at this point either.
--
T
EdColeman commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749373143
[Mockito](https://site.mockito.org/) may also be an alternative.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
dlmarion commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749364202
PowerMock is used to in 2 places, the unit tests for the Compactor and
CompactionCoordinator. I wonder if we can just delete these unit tests, we have
coverage on their functionality in
keith-turner commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749349321
> Looking at the PowerMock mailing list and GitHub, I think it might be
dead. Last commit back in Feb 2022, last release in 2020.
That is a good find from trying to build with
keith-turner commented on PR #3816:
URL: https://github.com/apache/accumulo/pull/3816#issuecomment-1749341591
Adding the test for bulk, merge, compaction, etc gives good high coverage of
the major features of Accumulo. The following test seem to exercise internal
impl details of the high l
dlmarion commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749334536
> > Currently at the point where tests using PowerMock are failing. I think
PowerMock might need to be ripped out at this point.
>
> What problem is powermock causing w/ jdk21? Li
keith-turner commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749327954
> Currently at the point where tests using PowerMock are failing. I think
PowerMock might need to be ripped out at this point.
What problem is powermock causing w/ jdk21? Lik
keith-turner commented on code in PR #3814:
URL: https://github.com/apache/accumulo/pull/3814#discussion_r1347729647
##
test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java:
##
@@ -1336,24 +1340,27 @@ public void importDirectoryOld() throws Exception {
File
dlmarion commented on PR #3810:
URL: https://github.com/apache/accumulo/pull/3810#issuecomment-1749299496
I fixed some of the errors in 99e7ecd. Currently at the point where tests
using PowerMock are failing. I think PowerMock might need to be ripped out at
this point.
--
This is an auto
keith-turner commented on issue #3741:
URL: https://github.com/apache/accumulo/issues/3741#issuecomment-1749291079
> I assume this means that scans in the ScanServer that were started before
the destructive operation started are safe to continue?
The scan got a snapshot of the tablets
keith-turner commented on code in PR #24:
URL: https://github.com/apache/accumulo-access/pull/24#discussion_r1347695822
##
src/test/java/org/apache/accumulo/access/AccessExpressionBenchmark.java:
##
@@ -117,8 +117,8 @@ List getStringExpressions() {
return allTestExpressio
keith-turner commented on code in PR #24:
URL: https://github.com/apache/accumulo-access/pull/24#discussion_r1347695050
##
src/test/java/org/apache/accumulo/access/AccessExpressionBenchmark.java:
##
@@ -117,8 +118,8 @@ List getStringExpressions() {
return allTestExpressio
dlmarion closed issue #3741: Avoid serving tablets with an operation id in the
scan server
URL: https://github.com/apache/accumulo/issues/3741
--
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 spe
dlmarion merged PR #3808:
URL: https://github.com/apache/accumulo/pull/3808
--
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...@ac
dlmarion commented on code in PR #3808:
URL: https://github.com/apache/accumulo/pull/3808#discussion_r1347633132
##
server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java:
##
@@ -285,6 +285,11 @@ public static boolean checkTabletMetadata(KeyExtent
exten
dlmarion commented on code in PR #3808:
URL: https://github.com/apache/accumulo/pull/3808#discussion_r1347632763
##
test/src/main/java/org/apache/accumulo/test/ScanServerIT.java:
##
@@ -246,6 +255,51 @@ public void testScanWithTabletHostingMix() throws
Exception {
}
}
DomGarguilo commented on code in PR #3770:
URL: https://github.com/apache/accumulo/pull/3770#discussion_r1347589050
##
server/monitor/src/main/appended-resources/META-INF/LICENSE:
##
@@ -75,14 +75,23 @@ Files:
* Dual licensed under the MIT and GPL licenses.
* http://
dlmarion commented on PR #3816:
URL: https://github.com/apache/accumulo/pull/3816#issuecomment-1749109616
FYI that I wanted to add the Sunny tag to `ServiceLockIT` as well, but I got
some error about the `Tag` that didn't make sense to me.
--
This is an automated message from the Apache G
dlmarion opened a new pull request, #3816:
URL: https://github.com/apache/accumulo/pull/3816
(no comment)
--
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-
dlmarion closed issue #3809: ScanServerIT.testScanWithTabletHostingMix is
failing
URL: https://github.com/apache/accumulo/issues/3809
--
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 com
dlmarion merged PR #3813:
URL: https://github.com/apache/accumulo/pull/3813
--
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...@ac
dlmarion closed issue #3783: Compactor should use ZooCache to get manager
address
URL: https://github.com/apache/accumulo/issues/3783
--
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 com
dlmarion commented on PR #3792:
URL: https://github.com/apache/accumulo/pull/3792#issuecomment-1748753978
> We don't want to be creating a situation where a partially upgraded
cluster (where we support less coordination for upgrades, as patch releases are
typically drop-in substitutes) can'
dlmarion merged PR #3812:
URL: https://github.com/apache/accumulo/pull/3812
--
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...@ac
dlmarion commented on code in PR #3813:
URL: https://github.com/apache/accumulo/pull/3813#discussion_r1347298150
##
core/src/main/java/org/apache/accumulo/core/util/compaction/ExternalCompactionUtil.java:
##
@@ -98,17 +98,12 @@ public static String getHostPortString(HostAndPort
63 matches
Mail list logo
