Re: Setting Charset in getBytes() call.

[Marc Parisi <ma...@accumulo.net>]

John, that would lead us to a configuration management issue. To keep
configuration files in line would be the same as ensuring file.encoding is
the same across the platform.

The JLS doesn't specify a Charset encoding scheme; however, for quite some
time the file.encoding fall through ( that is, when it's not specified ) is
UTF-8. This could change, and is not backed by the JLS, yet, file.encoding
is. It's a fallthrough, meant to take care of configuration mismanagement.

Further, these changes will have issues if you specify a file.encoding in
your configuration, as you don't always enforce UTF-8 in every String
instance, especially in some of your aggregator changes.

"If Accumulo was only a pile of servers, you could do this. You could
say that part of the configuration process for the servers is to
specify the desired encoding to file.encoding, and your shell scripts
could set UTF-8 by default.

But Accumulo is *not* just a pile of servers. Setting file.encoding
effects the entire JVM. A webapp that uses Accumulo now would need to
have the entire servlet container have a particular setting of
file.encoding. This just does not work in the wild. Even without the
servlet container issue, a user of Accumulo may need to plug it into
an existing code base that has other reasons to set file.encoding, and
will not like it when Accumulo starts to corrupt his or her string
data."

I gather that what you mean is that multiple, transient, execution paths
within the tserver should support multiple encodings; however, setting
file.encoding ensures that the platform, which is encompassed in a JVM,
encodes and decodes values in an understood way ( that's what character set
encodings are meant to enforce ). If a user wishes to have his or her own
execution path ( or their own encoding for an iterator ), then he/she would
likely define this. The fact that we require configuration parameters for
the bulk of these changes in core is an indication that the core API
contains features that are seeping into user functionality. Keep the
encoding/decoding at client code, not within the tserver process. Use
file.encoding for the core project, and our changeset would be much
smaller, require that clients do their own encoding/decoding.

A webapp is a fantastic example; however, let's take it a step further.
Accumulo is JBoss. The iterator/client code is the webapp. We should
separate Accumulo from client and client iterator code to avoid these
design issues and place the onus on the user, not accumulo. In all honesty,
and I'm probably off base, but in the case of iterators, we should move
them to a different package, and if so desired, add options to the
iterators, but there is no need to default to UTF-8. It's been that way for
some time.


On Wed, Oct 31, 2012 at 2:02 PM, Christopher Tubbs <ct...@gmail.com>wrote:

> I've added my own comments to this thread on the ACCUMULO-840 ticket.
>
> https://issues.apache.org/jira/browse/ACCUMULO-840?focusedCommentId=13488024&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13488024
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Tue, Oct 30, 2012 at 10:35 PM, John Vines <vi...@apache.org> wrote:
> > Why not just have a configuration in the xml file for setting a global
> > charset? This way we avoid hard coded settings but also avoid the issue
> of
> > shared vm issues.
> >
> > John
> >
> > Sent from my phone, pardon the typos and brevity.
> > On Oct 30, 2012 10:29 PM, "David Medinets" <da...@gmail.com>
> wrote:
> >
> >> Re-reading and re-thinking I can see your point about how, by
> >> specifying UTF-8, Accumulo is now flouting the file.encoding
> >> parameter. I'd like to implement a static method inside
> >> core/src/main/java/org/apache/accumulo/core/util/Encoding.java. Then
> >> do something like getBytes(Encoding.getCharset()) instead of
> >> hard-coding UTF-8.
> >>
> >> Class Encoding {
> >>   private static final Charset charset = null;
> >>   public Charset getCharset() {
> >>     if (charset == null) {
> >>       charset = Charset.forName(System.getProperty("file.encoding",
> >> "UTF-8"));
> >>     }
> >>     return charset;
> >>   }
> >>   ...
> >> }
> >>
>

Re: Setting Charset in getBytes() call.

[David Medinets <da...@gmail.com>]

Christopher, thanks for taking the time to writeup your comments. Nicely done.

Re: Setting Charset in getBytes() call.

[Christopher Tubbs <ct...@gmail.com>]

On Fri, Nov 2, 2012 at 3:56 PM, Benson Margulies <bi...@gmail.com> wrote:
> Maybe I'm being particularly dense, but I still think that this is
> being made too complex by failing to enumerate the specific goals.

I agree that there has been a failure to enumerate specific goals with
regard to encoding. I made an attempt to identify potential goals
(scopes), for which encoding matters on this ticket:
https://issues.apache.org/jira/browse/ACCUMULO-840

In there, I identify two considerations:
1) API issues addressing consistency for user data (eg. passwords,
table names, Mutation constructors that take Strings), and
2) INTERNAL issues related to Accumulo storing and reading state that
persists or is communicated between its operating components (a clear
example of this is how we store the !METADATA column family names,
which start out as Java String literals, and get encoded to bytes by
the time it gets stored in the table).

I think #1 can be addressed by simply waiting until somebody presents
a feature request with a use case, and in the meantime, we simply
don't touch it.

I think #2 can be addressed by establishing an internal policy (along
the lines of our codestyle standards) that establishes that Accumulo
will consistently store String data for its internal use as UTF8 when
we have to store that String as bytes, and when we convert such bytes
into Strings, we do so under the assumption it is UTF8. If we can
agree to this policy, anything that is actually non-compliant (i.e.
where there's a possibility it won't be stored or read as UTF8) will
simply be a bug that we apply a very narrowly scoped bugfix to ensure
consistency with the policy. I think David has already identified some
such cases and attempted to fix them in the process of working on
ACCUMULO-836. I think those are fine, but they need to be checked to
ensure that when they are converted back to a String, they are read as
UTF8. However, it might be better if these changes were split into
separate bugs, because even though they are all the same class of bug,
they apply to separate components (eg. "Potential bug - Inconsistent
encoding with Zookeeper data", "Potential bug - Inconsistent encoding
with mapreduce configuration", etc.). These bugs can be identified and
fixed as we encounter them, rather than as an attempt to fix the
entire code base. We shouldn't have to spend a lot of time on them...
we should do the simple thing first: establish the policy.

--
Christopher L Tubbs II
http://gravatar.com/ctubbsii

Re: Setting Charset in getBytes() call.

[John Vines <vi...@apache.org>]

Client/server mismatch is a giant problem. And the more combustibility we
put into Accumulo the closer we get to users hitting a knowledge barrier
about knowing the specifics of their Accumulo instance. i believe there are
two avenues for dealing with this-
1. Avoid at all costs. Unfortunately, this can ultimately boil down to
users losing features because we don't want them to have any sort of
intimate knowledge of the system.
2. A remote configuration utility. If we can have the client code pull the
configuration from the server, perhaps when Connection is made, we can have
our client APIs consistent on both sides of the channel. I believe a
solution like this could handle the issue Benson mentions, but it also
means we cannot approach this encoding issue with file.encoding.

Personally, I think the second option is an inevitability for us as we do
more and more features which are configuration specific. Either way, it
does seem that file.encoding is not sufficient as we want to avoid the
client code requiring some extremely specific documentation. it might even
be an incompatible configuration with what the client wants to do.

I think we are overgeneralizing this issue though. Josh did a decent job
and starting to hammer away on this. It's not just a matter of us doing
weird things with encodings, but the cases for them. For instance, all
zookeeper operations need to be done the same way across the board. This is
needs to be shared knowledge for both servers and clients. So these should
have charset specifications. But other things (pulling things out of thin
air), such as the client api methods, are outside of the purview. Primarily
because they are not associated with any tables until well after they are
created. So that is a user-space burden and should not be a concern with
us. Or any sort of local string operation.

It boils down to if it directly goes into HDFS, zookeeper, or the !METADATA
table then we should enforce encoding, in the way Dave approached it.
Outside of those scopes I think we should really just leave them the hell
alone because the system shouldn't be messing with user's data.

John


On Fri, Nov 2, 2012 at 3:56 PM, Benson Margulies <bi...@gmail.com>wrote:

> Maybe I'm being particularly dense, but I still think that this is
> being made too complex by failing to enumerate the specific goals.
>
> First case; data for which Accumulo is defined to persistently store
> *characters*, as opposed to bytes. I would hope that, in all such
> cases, we would agree that those characters should be stored in some
> Unicode format, never in some legacy encoding.
>
> Second case; data for which Accumulo is defined to store bytes, but,
> for convenience, an API allows the user to read and write characters.
> In this case, I can imagine two competing API designs. One would be to
> mirror Java, and in all such cases give the user the option of
> specifying the charset, defaulting to file.encoding. The other would
> be to insist on UTF-8. A third possibility - to just respect
> file.encoding - seems to me to be retreading the errors of Java 1.x.
>
> Third case; cases in which the user either supplies a text file for
> Accumulo to read, or asks Accumulo to write a text file. Having an API
> that can default to file.encoding here would be convenient for users,
> who want files in their platform's default encoding. Note that this is
> incompatible with the notion of *setting* file.encoding as an
> implementation technique for getting the string constructor and
> getBytes() to do UTF-8.
>
> Finally for today, I had a hard time following the response to my
> writing on servlets. I'll vastly simplify my presentation: when a user
> of Accumulo writes Java code that calls the Accumulo API, I find it
> unacceptable to require that user to set file.encoding to get correct
> behavior from Accumulo, except as described in the second case above.
> When Accumulo classes are integrated into user applications, Accumulo
> must respect file.encoding, or ignore file.encoding, but it cannot
> require the user to set it to something in particular to get correct
> behavior.
>

Re: Setting Charset in getBytes() call.

[Benson Margulies <bi...@gmail.com>]

Maybe I'm being particularly dense, but I still think that this is
being made too complex by failing to enumerate the specific goals.

First case; data for which Accumulo is defined to persistently store
*characters*, as opposed to bytes. I would hope that, in all such
cases, we would agree that those characters should be stored in some
Unicode format, never in some legacy encoding.

Second case; data for which Accumulo is defined to store bytes, but,
for convenience, an API allows the user to read and write characters.
In this case, I can imagine two competing API designs. One would be to
mirror Java, and in all such cases give the user the option of
specifying the charset, defaulting to file.encoding. The other would
be to insist on UTF-8. A third possibility - to just respect
file.encoding - seems to me to be retreading the errors of Java 1.x.

Third case; cases in which the user either supplies a text file for
Accumulo to read, or asks Accumulo to write a text file. Having an API
that can default to file.encoding here would be convenient for users,
who want files in their platform's default encoding. Note that this is
incompatible with the notion of *setting* file.encoding as an
implementation technique for getting the string constructor and
getBytes() to do UTF-8.

Finally for today, I had a hard time following the response to my
writing on servlets. I'll vastly simplify my presentation: when a user
of Accumulo writes Java code that calls the Accumulo API, I find it
unacceptable to require that user to set file.encoding to get correct
behavior from Accumulo, except as described in the second case above.
When Accumulo classes are integrated into user applications, Accumulo
must respect file.encoding, or ignore file.encoding, but it cannot
require the user to set it to something in particular to get correct
behavior.