[jira] [Updated] (ACCUMULO-4779) Instantiating iterator accesses config in a very slow way
[ https://issues.apache.org/jira/browse/ACCUMULO-4779?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Christopher Tubbs updated ACCUMULO-4779: Fix Version/s: 2.0.0 1.9.0 > Instantiating iterator accesses config in a very slow way > - > > Key: ACCUMULO-4779 > URL: https://issues.apache.org/jira/browse/ACCUMULO-4779 > Project: Accumulo > Issue Type: Bug >Affects Versions: 1.7.3, 1.8.1 >Reporter: Keith Turner >Assignee: Keith Turner > Fix For: 1.9.0, 2.0.0 > > > I noticed this will looking at profiling data. When creating iterators all > configuration is put in a TreeMap just to get two keys out. > The following code > https://github.com/apache/accumulo/blob/rel/1.8.1/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java#L118 > calls the following code > https://github.com/apache/accumulo/blob/rel/1.8.1/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java#L153 -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (ACCUMULO-4778) Resolving table name to table id is expensive
[ https://issues.apache.org/jira/browse/ACCUMULO-4778?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Christopher Tubbs updated ACCUMULO-4778: Fix Version/s: 2.0.0 > Resolving table name to table id is expensive > - > > Key: ACCUMULO-4778 > URL: https://issues.apache.org/jira/browse/ACCUMULO-4778 > Project: Accumulo > Issue Type: Bug >Affects Versions: 1.7.3, 1.8.1 >Reporter: Keith Turner > Fix For: 2.0.0 > > > I was running a Fluo test application and profiling the tablet server and > Fluo worker. The Fluo worker does lots small scans against Accumulo. > Resolving table names to ids (which is done for each scan) was expensive > enough to make a significant showing in the profiling data. > I looked that the 1.8 code and it does the following to resolve a table name : > * reads over all cached table ids in zookeeper putting them in a treemap > * does a lookup in the treemap > Ideally the client code would keep a cache of name to id mappings and > invalidate them when something changes in zookeeper. The data in zookeeper > is stored by id, so it does need to be inverted to lookup by name. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (ACCUMULO-4611) Commons Configuration either needs bumped or needs to be provided for Hadoop 3
[
https://issues.apache.org/jira/browse/ACCUMULO-4611?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Tubbs updated ACCUMULO-4611:
Status: Patch Available (was: In Progress)
> Commons Configuration either needs bumped or needs to be provided for Hadoop 3
> --
>
> Key: ACCUMULO-4611
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4611
> Project: Accumulo
> Issue Type: Bug
>Affects Versions: 1.8.1, 1.7.2
> Environment: CentOS 7
> Accumulo 1.7.x, 1.8.x
> Hadoop 3.0.0-alpha 2
> Zookeeper 3.4.9
>Reporter: Michael Hogue
>Assignee: Christopher Tubbs
>Priority: Critical
> Labels: pull-request-available
> Fix For: 1.9.0, 2.0.0
>
>
> I was investigating running either Accumulo 1.7.x or 1.8.x on Hadoop
> 3.0.0-alpha 2 and ran into a couple of issues. Since Accumulo assumes some
> dependencies will be provided by Hadoop (per ACCUMULO-1244), if those
> dependencies change then Accumulo might also need to change.
> HADOOP-13660 bumped {{commons-configuration}} from 1.6 -> 2.1 for Hadoop
> 3.0.0 and so NoClassDefFoundErrors are thrown per the below stack trace on
> {{accumulo init}}:
> {noformat}
> [mike@localhost cloud]$ accumulo init
> 2017-03-22 15:38:34,106 [start.Main] ERROR: Uncaught exception
> java.util.ServiceConfigurationError:
> org.apache.accumulo.start.spi.KeywordExecutable: Provider
> org.apache.accumulo.proxy.Proxy could not be instantiated
> at java.util.ServiceLoader.fail(ServiceLoader.java:232)
> at java.util.ServiceLoader.access$100(ServiceLoader.java:185)
> at
> java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:384)
> at java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
> at java.util.ServiceLoader$1.next(ServiceLoader.java:480)
> at org.apache.accumulo.start.Main.checkDuplicates(Main.java:223)
> at org.apache.accumulo.start.Main.getExecutables(Main.java:215)
> at org.apache.accumulo.start.Main.main(Main.java:78)
> Caused by: java.lang.NoClassDefFoundError:
> org/apache/commons/configuration/Configuration
> at java.lang.Class.getDeclaredConstructors0(Native Method)
> at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
> at java.lang.Class.getConstructor0(Class.java:3075)
> at java.lang.Class.newInstance(Class.java:412)
> at
> java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:380)
> ... 5 more
> Caused by: java.lang.ClassNotFoundException:
> org.apache.commons.configuration.Configuration
> at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
> at
> org.apache.accumulo.start.classloader.AccumuloClassLoader$2.loadClass(AccumuloClassLoader.java:284)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
> ... 10 more
> {noformat}
> I worked around this by dropping the {{commons-configuration}} 1.6 jar on
> Accumulo's classpath, but this should either be provided by Accumulo now or
> Accumulo should be bumped to {{commons-configuration2}} as well. Note that
> the latter change would cause problems for Hadoop 2 installations.
> There was actually one more thing that needed added to the accumulo-site.xml
> in order to get Accumulo to run in Hadoop 3:
> {noformat}
> $HADOOP_PREFIX/share/hadoop/client/[^.].*.jar
> {noformat}
> The reason for this is because {{Hdfs.java}} moved from the hadoop-hdfs jar
> to the hadoop-client-api jar.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Updated] (ACCUMULO-4611) Commons Configuration either needs bumped or needs to be provided for Hadoop 3
[
https://issues.apache.org/jira/browse/ACCUMULO-4611?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Tubbs updated ACCUMULO-4611:
Labels: pull-request-available (was: )
> Commons Configuration either needs bumped or needs to be provided for Hadoop 3
> --
>
> Key: ACCUMULO-4611
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4611
> Project: Accumulo
> Issue Type: Bug
>Affects Versions: 1.7.2, 1.8.1
> Environment: CentOS 7
> Accumulo 1.7.x, 1.8.x
> Hadoop 3.0.0-alpha 2
> Zookeeper 3.4.9
>Reporter: Michael Hogue
>Assignee: Christopher Tubbs
>Priority: Critical
> Labels: pull-request-available
> Fix For: 1.9.0, 2.0.0
>
>
> I was investigating running either Accumulo 1.7.x or 1.8.x on Hadoop
> 3.0.0-alpha 2 and ran into a couple of issues. Since Accumulo assumes some
> dependencies will be provided by Hadoop (per ACCUMULO-1244), if those
> dependencies change then Accumulo might also need to change.
> HADOOP-13660 bumped {{commons-configuration}} from 1.6 -> 2.1 for Hadoop
> 3.0.0 and so NoClassDefFoundErrors are thrown per the below stack trace on
> {{accumulo init}}:
> {noformat}
> [mike@localhost cloud]$ accumulo init
> 2017-03-22 15:38:34,106 [start.Main] ERROR: Uncaught exception
> java.util.ServiceConfigurationError:
> org.apache.accumulo.start.spi.KeywordExecutable: Provider
> org.apache.accumulo.proxy.Proxy could not be instantiated
> at java.util.ServiceLoader.fail(ServiceLoader.java:232)
> at java.util.ServiceLoader.access$100(ServiceLoader.java:185)
> at
> java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:384)
> at java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
> at java.util.ServiceLoader$1.next(ServiceLoader.java:480)
> at org.apache.accumulo.start.Main.checkDuplicates(Main.java:223)
> at org.apache.accumulo.start.Main.getExecutables(Main.java:215)
> at org.apache.accumulo.start.Main.main(Main.java:78)
> Caused by: java.lang.NoClassDefFoundError:
> org/apache/commons/configuration/Configuration
> at java.lang.Class.getDeclaredConstructors0(Native Method)
> at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
> at java.lang.Class.getConstructor0(Class.java:3075)
> at java.lang.Class.newInstance(Class.java:412)
> at
> java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:380)
> ... 5 more
> Caused by: java.lang.ClassNotFoundException:
> org.apache.commons.configuration.Configuration
> at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
> at
> org.apache.accumulo.start.classloader.AccumuloClassLoader$2.loadClass(AccumuloClassLoader.java:284)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
> ... 10 more
> {noformat}
> I worked around this by dropping the {{commons-configuration}} 1.6 jar on
> Accumulo's classpath, but this should either be provided by Accumulo now or
> Accumulo should be bumped to {{commons-configuration2}} as well. Note that
> the latter change would cause problems for Hadoop 2 installations.
> There was actually one more thing that needed added to the accumulo-site.xml
> in order to get Accumulo to run in Hadoop 3:
> {noformat}
> $HADOOP_PREFIX/share/hadoop/client/[^.].*.jar
> {noformat}
> The reason for this is because {{Hdfs.java}} moved from the hadoop-hdfs jar
> to the hadoop-client-api jar.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[GitHub] ctubbsii commented on issue #357: ACCUMULO-4611 Deprecate commons config in api
ctubbsii commented on issue #357: ACCUMULO-4611 Deprecate commons config in api URL: https://github.com/apache/accumulo/pull/357#issuecomment-357140022 @asfbot doesn't seem to be linking to this PR on https://issues.apache.org/jira/browse/ACCUMULO-4611 I'll add one manually. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ctubbsii commented on issue #357: ACCUMULO-4611 Deprecate commons config in api
ctubbsii commented on issue #357: ACCUMULO-4611 Deprecate commons config in api URL: https://github.com/apache/accumulo/pull/357#issuecomment-357139909 My mistake. Initially I did the PR against master instead of 1.8 and now Jenkins is reporting an error. I fixed it. It builds find against 1.8. Not sure how to force Jenkins to rerun, though. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
Accumulo-Pull-Requests - Build # 958 - Failure
The Apache Jenkins build system has built Accumulo-Pull-Requests (build #958) Status: Failure Check console output at https://builds.apache.org/job/Accumulo-Pull-Requests/958/ to view the results.
[GitHub] ctubbsii opened a new pull request #357: ACCUMULO-4611 Deprecate commons config in api
ctubbsii opened a new pull request #357: ACCUMULO-4611 Deprecate commons config in api URL: https://github.com/apache/accumulo/pull/357 In 3 commits: # Bump to version 1.9.0 as required by semver for API additions, and so we can deprecate and remove the offending API in 2.0.0 as discussed on the mailing list # Update apilyzer rules to find offending APIs with respect to commons-config # Fix The third "fix" commit is the most interesting one. If necessary, I can push the first two to a branch, and then then do a pull request with just the last commit against that branch, so it's easier to review on its own. Reviewers, let me know if you want me to do that. @joshelser Tagging you here for review, b/c GitHub won't show your GitHub user name in the "Request reviewers" dialog. Probably b/c you haven't sync'd with GitBox to be added to the repo? Not sure. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Updated] (ACCUMULO-4611) Commons Configuration either needs bumped or needs to be provided for Hadoop 3
[
https://issues.apache.org/jira/browse/ACCUMULO-4611?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Tubbs updated ACCUMULO-4611:
Fix Version/s: 1.9.0
> Commons Configuration either needs bumped or needs to be provided for Hadoop 3
> --
>
> Key: ACCUMULO-4611
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4611
> Project: Accumulo
> Issue Type: Bug
>Affects Versions: 1.7.2, 1.8.1
> Environment: CentOS 7
> Accumulo 1.7.x, 1.8.x
> Hadoop 3.0.0-alpha 2
> Zookeeper 3.4.9
>Reporter: Michael Hogue
>Assignee: Christopher Tubbs
>Priority: Critical
> Fix For: 1.9.0, 2.0.0
>
>
> I was investigating running either Accumulo 1.7.x or 1.8.x on Hadoop
> 3.0.0-alpha 2 and ran into a couple of issues. Since Accumulo assumes some
> dependencies will be provided by Hadoop (per ACCUMULO-1244), if those
> dependencies change then Accumulo might also need to change.
> HADOOP-13660 bumped {{commons-configuration}} from 1.6 -> 2.1 for Hadoop
> 3.0.0 and so NoClassDefFoundErrors are thrown per the below stack trace on
> {{accumulo init}}:
> {noformat}
> [mike@localhost cloud]$ accumulo init
> 2017-03-22 15:38:34,106 [start.Main] ERROR: Uncaught exception
> java.util.ServiceConfigurationError:
> org.apache.accumulo.start.spi.KeywordExecutable: Provider
> org.apache.accumulo.proxy.Proxy could not be instantiated
> at java.util.ServiceLoader.fail(ServiceLoader.java:232)
> at java.util.ServiceLoader.access$100(ServiceLoader.java:185)
> at
> java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:384)
> at java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
> at java.util.ServiceLoader$1.next(ServiceLoader.java:480)
> at org.apache.accumulo.start.Main.checkDuplicates(Main.java:223)
> at org.apache.accumulo.start.Main.getExecutables(Main.java:215)
> at org.apache.accumulo.start.Main.main(Main.java:78)
> Caused by: java.lang.NoClassDefFoundError:
> org/apache/commons/configuration/Configuration
> at java.lang.Class.getDeclaredConstructors0(Native Method)
> at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
> at java.lang.Class.getConstructor0(Class.java:3075)
> at java.lang.Class.newInstance(Class.java:412)
> at
> java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:380)
> ... 5 more
> Caused by: java.lang.ClassNotFoundException:
> org.apache.commons.configuration.Configuration
> at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
> at
> org.apache.accumulo.start.classloader.AccumuloClassLoader$2.loadClass(AccumuloClassLoader.java:284)
> at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
> ... 10 more
> {noformat}
> I worked around this by dropping the {{commons-configuration}} 1.6 jar on
> Accumulo's classpath, but this should either be provided by Accumulo now or
> Accumulo should be bumped to {{commons-configuration2}} as well. Note that
> the latter change would cause problems for Hadoop 2 installations.
> There was actually one more thing that needed added to the accumulo-site.xml
> in order to get Accumulo to run in Hadoop 3:
> {noformat}
> $HADOOP_PREFIX/share/hadoop/client/[^.].*.jar
> {noformat}
> The reason for this is because {{Hdfs.java}} moved from the hadoop-hdfs jar
> to the hadoop-client-api jar.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
Accumulo-Master - Build # 2223 - Fixed
The Apache Jenkins build system has built Accumulo-Master (build #2223) Status: Fixed Check console output at https://builds.apache.org/job/Accumulo-Master/2223/ to view the results.
Accumulo-Master - Build # 2222 - Unstable
The Apache Jenkins build system has built Accumulo-Master (build #) Status: Unstable Check console output at https://builds.apache.org/job/Accumulo-Master// to view the results.
[jira] [Resolved] (ACCUMULO-3902) Ensure [Batch]Scanners are closed in ITs
[ https://issues.apache.org/jira/browse/ACCUMULO-3902?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Christopher Tubbs resolved ACCUMULO-3902. - Resolution: Fixed > Ensure [Batch]Scanners are closed in ITs > > > Key: ACCUMULO-3902 > URL: https://issues.apache.org/jira/browse/ACCUMULO-3902 > Project: Accumulo > Issue Type: Bug > Components: test >Reporter: Josh Elser >Assignee: Jared R >Priority: Trivial > Labels: newbie, pull-request-available > Fix For: 2.0.0 > > Time Spent: 5h 40m > Remaining Estimate: 0h > > Do an audit of the integration tests to verify that we actually close > Scanners and BatchScanners. This is a best practice that we should be > encouraging (by doing it in our tests). It can also lead to bugs in other > test cases (ACCUMULO-3888). -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] ctubbsii commented on issue #356: ACCUMULO-4777 Removed the unused sequence generator.
ctubbsii commented on issue #356: ACCUMULO-4777 Removed the unused sequence generator. URL: https://github.com/apache/accumulo/pull/356#issuecomment-357111501 Good to know, but I think it's unlikely we'll reopen development on 1.6 to release a fix on that. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Resolved] (ACCUMULO-1975) Refactor AccumuloConfiguration.instantiateClassProperty and Property.createInstanceFromPropertyName
[ https://issues.apache.org/jira/browse/ACCUMULO-1975?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Christopher Tubbs resolved ACCUMULO-1975. - Resolution: Fixed > Refactor AccumuloConfiguration.instantiateClassProperty and > Property.createInstanceFromPropertyName > --- > > Key: ACCUMULO-1975 > URL: https://issues.apache.org/jira/browse/ACCUMULO-1975 > Project: Accumulo > Issue Type: Improvement >Reporter: Bill Havanki >Assignee: Jared R >Priority: Minor > Labels: newbie, pull-request-available > Fix For: 2.0.0 > > Time Spent: 50m > Remaining Estimate: 0h > > ... because they do the same thing. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] ivakegg commented on issue #355: ACCUMULO-4777 Removed the unused sequence generator.
ivakegg commented on issue #355: ACCUMULO-4777 Removed the unused sequence generator. URL: https://github.com/apache/accumulo/pull/355#issuecomment-357110060 Correct This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivakegg commented on issue #356: ACCUMULO-4777 Removed the unused sequence generator.
ivakegg commented on issue #356: ACCUMULO-4777 Removed the unused sequence generator. URL: https://github.com/apache/accumulo/pull/356#issuecomment-357109944 Also broken in 1.6.4 I believe This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ctubbsii commented on issue #355: ACCUMULO-4777 Removed the unused sequence generator.
ctubbsii commented on issue #355: ACCUMULO-4777 Removed the unused sequence generator. URL: https://github.com/apache/accumulo/pull/355#issuecomment-357107927 @ivakegg Assuming you closed this in order to open against 1.8 (#356) Is that correct? In future, if you click "Edit" next to the title, you can change the branch that this PR is pulled against, and do a force push to update your branch with the appropriate changeset... in case you didn't want to close and open a new issue in future. This is fine, too, though... it doesn't matter much. :smile_cat: This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (ACCUMULO-4777) Root tablet got spammed with 1.8 million log entries
[ https://issues.apache.org/jira/browse/ACCUMULO-4777?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16323254#comment-16323254 ] Ivan Bella commented on ACCUMULO-4777: -- [~kturner] As far as I can tell this sequence generator value is not actually being used anywhere. That may have been how it was used in the past, but no longer. I created a pull request that strips that out. > Root tablet got spammed with 1.8 million log entries > > > Key: ACCUMULO-4777 > URL: https://issues.apache.org/jira/browse/ACCUMULO-4777 > Project: Accumulo > Issue Type: Bug >Affects Versions: 1.8.1 >Reporter: Ivan Bella >Priority: Critical > Labels: pull-request-available > Fix For: 1.8.2, 2.0.0 > > Time Spent: 0.5h > Remaining Estimate: 0h > > We had a tserver that was handling accumulo.metadata tablets that somehow got > into a loop where it created over 22K empty wal logs. There were around 70 > metadata tablets and this resulted in around 1.8 million log entries in added > to the accumulo.root table. The only reason it stopped creating wal logs is > because it ran out of open file handles. This took us many hours and cups of > coffee to clean up. > The log contained the following messages in a tight loop: > log.TabletServerLogger INFO : Using next log hdfs://... > tserver.TabletServfer INFO : Writing log marker for hdfs://... > tserver.TabletServer INFO : Marking hdfs://... closed > log.DfsLogger INFO : Slow sync cost ... > ... > Unfortunately we did not have DEBUG turned on so we have no debug messages. > Tracking through the code there are three places where the > TabletServerLogger.close method is called: > 1) via resetLoggers in the TabletServerLogger, but nothing calls this method > so this is ruled out > 2) when the log gets too large or too old, but neither of those checks should > have been hitting here. > 3) In a loop that is executed (while (!success)) in the > TabletServerLogger.write method. In this case when we unsuccessfullty write > something to the wal, then that one is closed and a new one is created. This > loop will go forever until we successfully write out the entry. A > DfsLogger.LogClosedException seems the most logical reason. This is most > likely because a ClosedChannelException was thrown from the DfsLogger.write > methods (around line 609 in DfsLogger). > So the root cause was most likely hadoop related. However in accumulo we > probably should not be doing a tight retry loop around a hadoop failure. I > recommend at a minimum doing some sort of exponential back off and perhaps > setting a limit on the number of retries resulting in a critical tserver > failure. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (ACCUMULO-4777) Root tablet got spammed with 1.8 million log entries
[ https://issues.apache.org/jira/browse/ACCUMULO-4777?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] ASF GitHub Bot updated ACCUMULO-4777: - Labels: pull-request-available (was: ) > Root tablet got spammed with 1.8 million log entries > > > Key: ACCUMULO-4777 > URL: https://issues.apache.org/jira/browse/ACCUMULO-4777 > Project: Accumulo > Issue Type: Bug >Affects Versions: 1.8.1 >Reporter: Ivan Bella >Priority: Critical > Labels: pull-request-available > Fix For: 1.8.2, 2.0.0 > > > We had a tserver that was handling accumulo.metadata tablets that somehow got > into a loop where it created over 22K empty wal logs. There were around 70 > metadata tablets and this resulted in around 1.8 million log entries in added > to the accumulo.root table. The only reason it stopped creating wal logs is > because it ran out of open file handles. This took us many hours and cups of > coffee to clean up. > The log contained the following messages in a tight loop: > log.TabletServerLogger INFO : Using next log hdfs://... > tserver.TabletServfer INFO : Writing log marker for hdfs://... > tserver.TabletServer INFO : Marking hdfs://... closed > log.DfsLogger INFO : Slow sync cost ... > ... > Unfortunately we did not have DEBUG turned on so we have no debug messages. > Tracking through the code there are three places where the > TabletServerLogger.close method is called: > 1) via resetLoggers in the TabletServerLogger, but nothing calls this method > so this is ruled out > 2) when the log gets too large or too old, but neither of those checks should > have been hitting here. > 3) In a loop that is executed (while (!success)) in the > TabletServerLogger.write method. In this case when we unsuccessfullty write > something to the wal, then that one is closed and a new one is created. This > loop will go forever until we successfully write out the entry. A > DfsLogger.LogClosedException seems the most logical reason. This is most > likely because a ClosedChannelException was thrown from the DfsLogger.write > methods (around line 609 in DfsLogger). > So the root cause was most likely hadoop related. However in accumulo we > probably should not be doing a tight retry loop around a hadoop failure. I > recommend at a minimum doing some sort of exponential back off and perhaps > setting a limit on the number of retries resulting in a critical tserver > failure. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] ivakegg opened a new pull request #356: ACCUMULO-4777 Removed the unused sequence generator.
ivakegg opened a new pull request #356: ACCUMULO-4777 Removed the unused sequence generator. URL: https://github.com/apache/accumulo/pull/356 This commit removed the unused sequence generator. Yet to come is adding a retry backoff strategy to the WAL writing mechanism. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ivakegg closed pull request #355: ACCUMULO-4777 Removed the unused sequence generator.
ivakegg closed pull request #355: ACCUMULO-4777 Removed the unused sequence
generator.
URL: https://github.com/apache/accumulo/pull/355
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
index b4c6cb8c0a..d32b6d12fc 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
@@ -95,8 +95,6 @@
// Use a ReadWriteLock to allow multiple threads to use the log set, but
obtain a write lock to change them
private final ReentrantReadWriteLock logIdLock = new
ReentrantReadWriteLock();
- private final AtomicInteger seqGen = new AtomicInteger();
-
private final AtomicLong syncCounter;
private final AtomicLong flushCounter;
@@ -348,19 +346,18 @@ synchronized private void close() throws IOException {
}
interface Writer {
-LoggerOperation write(DfsLogger logger, int seq) throws Exception;
+LoggerOperation write(DfsLogger logger) throws Exception;
}
- private int write(CommitSession commitSession, boolean mincFinish, Writer
writer) throws IOException {
+ private void write(CommitSession commitSession, boolean mincFinish, Writer
writer) throws IOException {
List sessions = Collections.singletonList(commitSession);
-return write(sessions, mincFinish, writer);
+write(sessions, mincFinish, writer);
}
- private int write(final Collection sessions, boolean
mincFinish, Writer writer) throws IOException {
+ private void write(final Collection sessions, boolean
mincFinish, Writer writer) throws IOException {
// Work very hard not to lock this during calls to the outside world
int currentLogId = logId.get();
-int seq = -1;
int attempt = 1;
boolean success = false;
while (!success) {
@@ -400,10 +397,7 @@ private int write(final Collection
sessions, boolean mincFinish,
if (currentLogId == logId.get()) {
// write the mutation to the logs
- seq = seqGen.incrementAndGet();
- if (seq < 0)
-throw new RuntimeException("Logger sequence generator wrapped!
Onos!!!11!eleven");
- LoggerOperation lop = writer.write(copy, seq);
+ LoggerOperation lop = writer.write(copy);
lop.await();
// double-check: did the log set change?
@@ -453,42 +447,40 @@ void withWriteLock() throws IOException {
closeForReplication(sessions);
}
});
-return seq;
}
protected void closeForReplication(Collection sessions) {
// TODO We can close the WAL here for replication purposes
}
- public int defineTablet(final CommitSession commitSession) throws
IOException {
+ public void defineTablet(final CommitSession commitSession) throws
IOException {
// scribble this into the metadata tablet, too.
-return write(commitSession, false, new Writer() {
+write(commitSession, false, new Writer() {
@Override
- public LoggerOperation write(DfsLogger logger, int ignored) throws
Exception {
+ public LoggerOperation write(DfsLogger logger) throws Exception {
logger.defineTablet(commitSession.getWALogSeq(),
commitSession.getLogId(), commitSession.getExtent());
return DfsLogger.NO_WAIT_LOGGER_OP;
}
});
}
- public int log(final CommitSession commitSession, final int tabletSeq, final
Mutation m, final Durability durability) throws IOException {
+ public void log(final CommitSession commitSession, final int tabletSeq,
final Mutation m, final Durability durability) throws IOException {
if (durability == Durability.NONE) {
- return -1;
+ return;
}
if (durability == Durability.DEFAULT) {
throw new IllegalArgumentException("Unexpected durability " +
durability);
}
-int seq = write(commitSession, false, new Writer() {
+write(commitSession, false, new Writer() {
@Override
- public LoggerOperation write(DfsLogger logger, int ignored) throws
Exception {
+ public LoggerOperation write(DfsLogger logger) throws Exception {
return logger.log(tabletSeq, commitSession.getLogId(), m, durability);
}
});
logSizeEstimate.addAndGet(m.numBytes());
-return seq;
}
- public int logManyTablets(Map mutations) throws
IOException {
+ public void logManyTablets(Map mutations) throws
IOException {
final Map loggables = new HashMap<>(mutations);
for (Entry entry :
[GitHub] ivakegg opened a new pull request #355: ACCUMULO-4777 Removed the unused sequence generator.
ivakegg opened a new pull request #355: ACCUMULO-4777 Removed the unused sequence generator. URL: https://github.com/apache/accumulo/pull/355 This commit removed the unused sequence generator. Yet to come is adding a retry backoff strategy to the WAL writing mechanism. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ctubbsii closed pull request #351: ACCUMULO-1975 Refactor Property.createInstanceFromPropertyName into AccumuloConfiguration.instantiateClassProperty
ctubbsii closed pull request #351: ACCUMULO-1975 Refactor
Property.createInstanceFromPropertyName into
AccumuloConfiguration.instantiateClassProperty
URL: https://github.com/apache/accumulo/pull/351
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
b/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
index 2c53407edb..832bebdd93 100644
---
a/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
+++
b/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
@@ -27,7 +27,6 @@
import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.conf.PropertyType.PortRange;
import org.apache.accumulo.core.util.Pair;
-import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -284,35 +283,4 @@ public int getMaxFilesPerTablet() {
*/
public void invalidateCache() {}
- /**
- * Creates a new instance of a class specified in a configuration property.
- *
- * @param property
- * property specifying class name
- * @param base
- * base class of type
- * @param defaultInstance
- * instance to use if creation fails
- * @return new class instance, or default instance if creation failed
- * @see AccumuloVFSClassLoader
- */
- public T instantiateClassProperty(Property property, Class base, T
defaultInstance) {
-String clazzName = get(property);
-T instance = null;
-
-try {
- Class clazz = AccumuloVFSClassLoader.loadClass(clazzName,
base);
- instance = clazz.newInstance();
- log.info("Loaded class : {}", clazzName);
-} catch (Exception e) {
- log.warn("Failed to load class ", e);
-}
-
-if (instance == null) {
- log.info("Using {}", defaultInstance.getClass().getName());
- instance = defaultInstance;
-}
-return instance;
- }
-
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
index 1918646aa0..b859e3f3db 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java
@@ -33,6 +33,7 @@
import org.apache.accumulo.core.client.impl.thrift.SecurityErrorCode;
import org.apache.accumulo.core.client.impl.thrift.ThriftSecurityException;
import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
+import org.apache.accumulo.core.conf.AccumuloConfiguration;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.conf.SiteConfiguration;
import org.apache.accumulo.core.data.thrift.IterInfo;
@@ -89,21 +90,23 @@ public static synchronized SecurityOperation
getInstance(AccumuloServerContext c
}
protected static Authorizor getAuthorizor(String instanceId, boolean
initialize) {
-Authorizor toRet =
SiteConfiguration.getInstance().instantiateClassProperty(Property.INSTANCE_SECURITY_AUTHORIZOR,
Authorizor.class,
-ZKAuthorizor.getInstance());
+AccumuloConfiguration conf = SiteConfiguration.getInstance();
+Authorizor toRet = Property.createInstanceFromPropertyName(conf,
Property.INSTANCE_SECURITY_AUTHORIZOR, Authorizor.class,
ZKAuthorizor.getInstance());
toRet.initialize(instanceId, initialize);
return toRet;
}
protected static Authenticator getAuthenticator(String instanceId, boolean
initialize) {
-Authenticator toRet =
SiteConfiguration.getInstance().instantiateClassProperty(Property.INSTANCE_SECURITY_AUTHENTICATOR,
Authenticator.class,
+AccumuloConfiguration conf = SiteConfiguration.getInstance();
+Authenticator toRet = Property.createInstanceFromPropertyName(conf,
Property.INSTANCE_SECURITY_AUTHENTICATOR, Authenticator.class,
ZKAuthenticator.getInstance());
toRet.initialize(instanceId, initialize);
return toRet;
}
protected static PermissionHandler getPermHandler(String instanceId, boolean
initialize) {
-PermissionHandler toRet =
SiteConfiguration.getInstance().instantiateClassProperty(Property.INSTANCE_SECURITY_PERMISSION_HANDLER,
PermissionHandler.class,
+AccumuloConfiguration conf = SiteConfiguration.getInstance();
+PermissionHandler toRet = Property.createInstanceFromPropertyName(conf,
Property.INSTANCE_SECURITY_PERMISSION_HANDLER, PermissionHandler.class,
ZKPermHandler.getInstance());
toRet.initialize(instanceId, initialize);
return toRet;
diff --git
[jira] [Updated] (ACCUMULO-4779) Instantiating iterator accesses config in a very slow way
[ https://issues.apache.org/jira/browse/ACCUMULO-4779?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Keith Turner updated ACCUMULO-4779: --- Summary: Instantiating iterator accesses config in a very slow way (was: Instantiating iterator access config in a very slow way) > Instantiating iterator accesses config in a very slow way > - > > Key: ACCUMULO-4779 > URL: https://issues.apache.org/jira/browse/ACCUMULO-4779 > Project: Accumulo > Issue Type: Bug >Affects Versions: 1.7.3, 1.8.1 >Reporter: Keith Turner >Assignee: Keith Turner > > I noticed this will looking at profiling data. When creating iterators all > configuration is put in a TreeMap just to get two keys out. > The following code > https://github.com/apache/accumulo/blob/rel/1.8.1/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java#L118 > calls the following code > https://github.com/apache/accumulo/blob/rel/1.8.1/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java#L153 -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Assigned] (ACCUMULO-4779) Instantiating iterator access config in a very slow way
[ https://issues.apache.org/jira/browse/ACCUMULO-4779?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Keith Turner reassigned ACCUMULO-4779: -- Assignee: Keith Turner > Instantiating iterator access config in a very slow way > --- > > Key: ACCUMULO-4779 > URL: https://issues.apache.org/jira/browse/ACCUMULO-4779 > Project: Accumulo > Issue Type: Bug >Affects Versions: 1.7.3, 1.8.1 >Reporter: Keith Turner >Assignee: Keith Turner > > I noticed this will looking at profiling data. When creating iterators all > configuration is put in a TreeMap just to get two keys out. > The following code > https://github.com/apache/accumulo/blob/rel/1.8.1/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java#L118 > calls the following code > https://github.com/apache/accumulo/blob/rel/1.8.1/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java#L153 -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] keith-turner commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
keith-turner commented on a change in pull request #347: ACCUMULO-4746 Fluent
API for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r161058786
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -183,6 +185,190 @@ private Value nv(String s) {
return new Value(s.getBytes());
}
+ @Test
+ public void testAtFamilyTypes() {
+final String fam = "f16bc";
+final String qual = "q1pm2";
+final String val = "v8672194923750";
+
+Mutation expected = new Mutation("row5");
+expected.put(fam, qual, val);
+
+// Test all family methods, keeping qual and val constant as Strings
+// fam: byte[]
+Mutation actual = new Mutation("row5");
+actual.at().family(fam.getBytes(UTF_8)).qualifier(qual).set(val);
Review comment:
> That has the indirect benefit of preventing unintended changes to the
existing public API of Mutation.
In general we should be using tools to check for API changes between
releases.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] keith-turner commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
keith-turner commented on a change in pull request #347: ACCUMULO-4746 Fluent
API for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r161058495
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -183,6 +185,190 @@ private Value nv(String s) {
return new Value(s.getBytes());
}
+ @Test
+ public void testAtFamilyTypes() {
+final String fam = "f16bc";
+final String qual = "q1pm2";
+final String val = "v8672194923750";
+
+Mutation expected = new Mutation("row5");
+expected.put(fam, qual, val);
+
+// Test all family methods, keeping qual and val constant as Strings
+// fam: byte[]
+Mutation actual = new Mutation("row5");
+actual.at().family(fam.getBytes(UTF_8)).qualifier(qual).set(val);
Review comment:
> Less junk in one Java file.
The new interfaces and implementation may not have to be in the the Mutation
file. The only thing that has to be there is the `at()` method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Created] (ACCUMULO-4778) Resolving table name to table id is expensive
Keith Turner created ACCUMULO-4778: -- Summary: Resolving table name to table id is expensive Key: ACCUMULO-4778 URL: https://issues.apache.org/jira/browse/ACCUMULO-4778 Project: Accumulo Issue Type: Bug Affects Versions: 1.8.1, 1.7.3 Reporter: Keith Turner I was running a Fluo test application and profiling the tablet server and Fluo worker. The Fluo worker does lots small scans against Accumulo. Resolving table names to ids (which is done for each scan) was expensive enough to make a significant showing in the profiling data. I looked that the 1.8 code and it does the following to resolve a table name : * reads over all cached table ids in zookeeper putting them in a treemap * does a lookup in the treemap Ideally the client code would keep a cache of name to id mappings and invalidate them when something changes in zookeeper. The data in zookeeper is stored by id, so it does need to be inverted to lookup by name. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (ACCUMULO-4777) Root tablet got spammed with 1.8 million log entries
[ https://issues.apache.org/jira/browse/ACCUMULO-4777?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16322739#comment-16322739 ] Ivan Bella commented on ACCUMULO-4777: -- After several days of getting my head around this code, I think I figured it out. There is a AtomicInteger used as a sequence counter in the TabletServerLogger. When this sequence counter wraps (goes negative), an exception is thrown. However in the write method where it is thrown, it will subsequently close the current WAL, open a new one, and recursively call itself via the defineTablet method. This underlying call will fail for the same reason, and then close the WAL, and recursively call it self again...etc, etc, etc. So basically we have tablet servers that have been up long enough to actually incur over 2^31 writes into the WALs. Once this happens, the server will go into this loop. I am guessing that not many systems leave the tablet servers up long enough for this to happen. Also, this is happening for us on tservers for which only the accumulo.metadata is pinned (via the HostRegexBalancer). Hence it is actually more likely to happen first on these tservers. As far as I can tell, every path to this write method basically ignores the sequence number returned. So what is the real purpose of this sequence generator? I think I need to original authors of this code to tell me. My inclination is to basically reset the sequence generator back to 0 and just continue. Any thoughts out there on this? > Root tablet got spammed with 1.8 million log entries > > > Key: ACCUMULO-4777 > URL: https://issues.apache.org/jira/browse/ACCUMULO-4777 > Project: Accumulo > Issue Type: Bug >Affects Versions: 1.8.1 >Reporter: Ivan Bella >Priority: Critical > Fix For: 1.8.2, 2.0.0 > > > We had a tserver that was handling accumulo.metadata tablets that somehow got > into a loop where it created over 22K empty wal logs. There were around 70 > metadata tablets and this resulted in around 1.8 million log entries in added > to the accumulo.root table. The only reason it stopped creating wal logs is > because it ran out of open file handles. This took us many hours and cups of > coffee to clean up. > The log contained the following messages in a tight loop: > log.TabletServerLogger INFO : Using next log hdfs://... > tserver.TabletServfer INFO : Writing log marker for hdfs://... > tserver.TabletServer INFO : Marking hdfs://... closed > log.DfsLogger INFO : Slow sync cost ... > ... > Unfortunately we did not have DEBUG turned on so we have no debug messages. > Tracking through the code there are three places where the > TabletServerLogger.close method is called: > 1) via resetLoggers in the TabletServerLogger, but nothing calls this method > so this is ruled out > 2) when the log gets too large or too old, but neither of those checks should > have been hitting here. > 3) In a loop that is executed (while (!success)) in the > TabletServerLogger.write method. In this case when we unsuccessfullty write > something to the wal, then that one is closed and a new one is created. This > loop will go forever until we successfully write out the entry. A > DfsLogger.LogClosedException seems the most logical reason. This is most > likely because a ClosedChannelException was thrown from the DfsLogger.write > methods (around line 609 in DfsLogger). > So the root cause was most likely hadoop related. However in accumulo we > probably should not be doing a tight retry loop around a hadoop failure. I > recommend at a minimum doing some sort of exponential back off and perhaps > setting a limit on the number of retries resulting in a critical tserver > failure. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[GitHub] joshelser commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
joshelser commented on a change in pull request #347: ACCUMULO-4746 Fluent API
for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r161012818
##
File path: core/src/main/java/org/apache/accumulo/core/data/Mutation.java
##
@@ -772,6 +772,474 @@ public void putDelete(byte[] columnFamily, byte[]
columnQualifier, ColumnVisibil
put(columnFamily, columnQualifier, columnVisibility.getExpression(), true,
timestamp, true, EMPTY_BYTES);
}
+ /**
+ * Fluent API for Mutation
+ *
+ *
+ * Each step of the Mutation is represented by an interface. Inheritance
hierarchy ensures that chained methods follow prescribed order of: family,
qualifier,
+ * visibility, timestamp.
+ *
+ *
+ * Methods are optional, meaning that one or may be omitted within a chain.
+ *
+ *
+ * Set and delete methods end the chain and finalize the mutation by filling
Mutation's UnsynchronizedBuffer.
+ */
+
+ /**
+ * Provides methods for setting the column family of a Mutation. The user
can provide the family name as a byte array, CharSequence, ByteBuffer, or Text
+ * object instance and the backend will do the necessary transformation.
+ *
+ * All FamilyStep methods return an instance derived from the QualifierStep
interface, allowing the methods to be semantically chained.
+ *
+ * @since 2.0
+ */
+ public interface FamilyStep extends QualifierStep {
+QualifierStep family(byte[] colFam);
+
+QualifierStep family(ByteBuffer colFam);
+
+QualifierStep family(CharSequence colFam);
+
+QualifierStep family(Text colFam);
+ }
+
+ /**
+ * Provides methods for setting the column qualifier of a Mutation. The user
can provide the qualifier name as a byte array, CharSequence, ByteBuffer, or
Text
+ * object instance and the backend will do the necessary transformation.
+ *
+ * All QualifierStep methods return an instance derived from the
VisibilityStep interface, allowing the methods to be semantically chained.
+ *
+ * @since 2.0
+ */
+ public interface QualifierStep extends VisibilityStep {
+VisibilityStep qualifier(byte[] colQual);
+
+VisibilityStep qualifier(ByteBuffer colQual);
+
+VisibilityStep qualifier(CharSequence colQual);
+
+VisibilityStep qualifier(Text colQual);
+ }
+
+ /**
+ * Provides methods for setting the column visibility of a Mutation. The
user can provide the visibility as a byte array or
+ * {@link org.apache.accumulo.core.security.ColumnVisibility} object
instance and the backend will do the necessary transformation.
+ *
+ * All QualifierStep methods return an instance derived from the
VisibilityStep interface, allowing the methods to be semantically chained.
+ *
+ * @since 2.0
+ */
+ public interface VisibilityStep extends TimestampStep {
+TimestampStep visibility(byte[] colVis);
+
+TimestampStep visibility(ByteBuffer colVis);
+
+TimestampStep visibility(CharSequence colVis);
+
+TimestampStep visibility(ColumnVisibility colVis);
+
+TimestampStep visibility(Text colVis);
+ }
+
+ /**
+ * Provides methods for setting the timestamp of a Mutation. The user must
provide the timestamp as a long.
+ *
+ * All TimestampStep methods return an instance derived from the
MutationStep interface, allowing the methods to be semantically chained.
+ *
+ * @since 2.0
+ */
+ public interface TimestampStep extends MutationStep {
+MutationStep timestamp(long ts);
+ }
+
+ /**
+ * Provides methods for setting the value of a Mutation. The user can
provide the value as a byte array, Value, or ByteBuffer object instance and the
backend
+ * will do the necessary transformation.
+ *
+ * All MutationStep methods complete a fluent Mutation API method chain.
+ *
+ * @since 2.0
+ */
+ public interface MutationStep {
+Mutation set(byte[] val);
+
+Mutation set(ByteBuffer val);
+
+Mutation set(CharSequence val);
+
+Mutation set(Text val);
+
+Mutation set(Value val);
+
+Mutation delete();
+ }
+
+ /**
+ * Returns a new FamilyStep object allowing the user to define changes to
the Mutation.
+ *
+ * @return a new FamilyStep object, advancing the method chain
+ * @since 2.0
+ */
+ public FamilyStep at() {
Review comment:
I'm also curious about this choice of name (after seeing some of the other
suggestions from the JIRA comments).
`add()` was the first thing I thought of. But, Mutation is essentially a map
of "column updates" for a row. `put()` may make more sense (aligning with the
underlying implementation -- for multiple updates to the same column in a
Mutation, I think the last update would "win").
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[GitHub] joshelser commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
joshelser commented on a change in pull request #347: ACCUMULO-4746 Fluent API
for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r161012119
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -183,6 +185,190 @@ private Value nv(String s) {
return new Value(s.getBytes());
}
+ @Test
+ public void testAtFamilyTypes() {
+final String fam = "f16bc";
+final String qual = "q1pm2";
+final String val = "v8672194923750";
+
+Mutation expected = new Mutation("row5");
+expected.put(fam, qual, val);
+
+// Test all family methods, keeping qual and val constant as Strings
+// fam: byte[]
+Mutation actual = new Mutation("row5");
+actual.at().family(fam.getBytes(UTF_8)).qualifier(qual).set(val);
Review comment:
> I was puzzling over this question and shared what I came up with on the
issue.
Thanks for the context, Keith.
> As long as the API call stays streamlined like below and the user doesn't
have to necessarily be aware of this new class in order to make modifications
on their Mutation.
Ya, that's what I was thinking. Less junk in one Java file. That has the
indirect benefit of preventing unintended changes to the existing public API of
Mutation.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] bfach10 commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
bfach10 commented on a change in pull request #347: ACCUMULO-4746 Fluent API
for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r161008327
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -183,6 +185,190 @@ private Value nv(String s) {
return new Value(s.getBytes());
}
+ @Test
+ public void testAtFamilyTypes() {
+final String fam = "f16bc";
+final String qual = "q1pm2";
+final String val = "v8672194923750";
+
+Mutation expected = new Mutation("row5");
+expected.put(fam, qual, val);
+
+// Test all family methods, keeping qual and val constant as Strings
+// fam: byte[]
+Mutation actual = new Mutation("row5");
+actual.at().family(fam.getBytes(UTF_8)).qualifier(qual).set(val);
Review comment:
Since the goal (in my view at least) is to have the method chain be easy to
use semantically for the caller, using a new class could be a way to go. As
long as the API call stays streamlined like below and the user doesn't have to
necessarily be aware of this new class in order to make modifications on their
`Mutation`.
`m.at().family().qualifier().visibility().set()`
So in the case of a new class the `at` method could return a new class
instead of the nested interface class it's returning now.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] keith-turner commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
keith-turner commented on a change in pull request #347: ACCUMULO-4746 Fluent
API for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r161006729
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -187,7 +187,8 @@ private Value nv(String s) {
public void testPuts() {
Mutation m = new Mutation(new Text("r1"));
-m.put(nt("cf1"), nt("cq1"), nv("v1"));
+m.at().family(nt("cf1")).qualifier(nt("cq1")).set(nv("v1"));
Review comment:
Where were you thinking of adding the `conditionalSet()` method?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] keith-turner commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
keith-turner commented on a change in pull request #347: ACCUMULO-4746 Fluent
API for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r161005568
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -183,6 +185,190 @@ private Value nv(String s) {
return new Value(s.getBytes());
}
+ @Test
+ public void testAtFamilyTypes() {
+final String fam = "f16bc";
+final String qual = "q1pm2";
+final String val = "v8672194923750";
+
+Mutation expected = new Mutation("row5");
+expected.put(fam, qual, val);
+
+// Test all family methods, keeping qual and val constant as Strings
+// fam: byte[]
+Mutation actual = new Mutation("row5");
+actual.at().family(fam.getBytes(UTF_8)).qualifier(qual).set(val);
Review comment:
> Is there a reason to not make the fluent-methods you want to add on a new
class, e.g. MutationBuilder?
I was puzzling over this question and shared what I came up with on the
issue.
https://issues.apache.org/jira/browse/ACCUMULO-4746?focusedCommentId=16269423=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16269423
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] bfach10 commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
bfach10 commented on a change in pull request #347: ACCUMULO-4746 Fluent API
for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r160999000
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -187,7 +187,8 @@ private Value nv(String s) {
public void testPuts() {
Mutation m = new Mutation(new Text("r1"));
-m.put(nt("cf1"), nt("cq1"), nv("v1"));
+m.at().family(nt("cf1")).qualifier(nt("cq1")).set(nv("v1"));
Review comment:
Idea: add a new group of `set` methods called `conditionalSet` that return a
`ConditionalMutation` object. This will enable users to continue the chain and
add conditions as well.
`public ConditionalMutation conditionalSet(byte[] val) { }`
`ConditionalMutation actual = new ConditionalMutation("row5");
actual.at().family("fam123").qualifier("q77").visibility(cv)
.conditionalSet("v4224").addCondition(new Condition())`
But doing something like this for each subclass could get messy. :/
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] joshelser commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
joshelser commented on a change in pull request #347: ACCUMULO-4746 Fluent API
for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r161001561
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -183,6 +185,190 @@ private Value nv(String s) {
return new Value(s.getBytes());
}
+ @Test
+ public void testAtFamilyTypes() {
+final String fam = "f16bc";
+final String qual = "q1pm2";
+final String val = "v8672194923750";
+
+Mutation expected = new Mutation("row5");
+expected.put(fam, qual, val);
+
+// Test all family methods, keeping qual and val constant as Strings
+// fam: byte[]
+Mutation actual = new Mutation("row5");
+actual.at().family(fam.getBytes(UTF_8)).qualifier(qual).set(val);
Review comment:
Oh durr. Sorry, I was thinking about ServerMutation (I think is the name of
it). Are you aware of that sub-class?
Is there a reason to not make the fluent-methods you want to add on a new
class, e.g. MutationBuilder?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] bfach10 commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
bfach10 commented on a change in pull request #347: ACCUMULO-4746 Fluent API
for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r160999000
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -187,7 +187,8 @@ private Value nv(String s) {
public void testPuts() {
Mutation m = new Mutation(new Text("r1"));
-m.put(nt("cf1"), nt("cq1"), nv("v1"));
+m.at().family(nt("cf1")).qualifier(nt("cq1")).set(nv("v1"));
Review comment:
Idea: add a new group of `set` methods called `conditionalSet` that return a
`ConditionalMutation` object. This will enable users to continue the chain and
add conditions as well.
`public ConditionalMutation conditionalSet(byte[] val) { }`
`ConditionalMutation actual = new ConditionalMutation("row5");
actual.at().family("fam123").qualifier("q77").visibility(cv)
.conditionalSet("v4224").addCondition(new Condition())`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] bfach10 commented on a change in pull request #347: ACCUMULO-4746 Fluent API for Mutation
bfach10 commented on a change in pull request #347: ACCUMULO-4746 Fluent API
for Mutation
URL: https://github.com/apache/accumulo/pull/347#discussion_r160973746
##
File path: core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
##
@@ -183,6 +185,190 @@ private Value nv(String s) {
return new Value(s.getBytes());
}
+ @Test
+ public void testAtFamilyTypes() {
+final String fam = "f16bc";
+final String qual = "q1pm2";
+final String val = "v8672194923750";
+
+Mutation expected = new Mutation("row5");
+expected.put(fam, qual, val);
+
+// Test all family methods, keeping qual and val constant as Strings
+// fam: byte[]
+Mutation actual = new Mutation("row5");
+actual.at().family(fam.getBytes(UTF_8)).qualifier(qual).set(val);
Review comment:
To make `Mutation` immutable we'd have to remove the `put()` methods since
those allow us to change the `Mutation`'s `UnsynchronizedBuffer`. We'd also
have to remove `ConditionalMutation`'s ability to add conditions.
The consensus in the JIRA ticket and PR comments seems to be that `row`
should be immutable but other aspects are fair game to change after object
creation.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
