[jira] [Comment Edited] (SLING-3737) Instance Sling Identifier may be randomly reset on restart

["Timothee Maret (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-3737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14054906#comment-14054906 ] 

Timothee Maret edited comment on SLING-3737 at 7/8/14 12:58 PM:
----------------------------------------------------------------

bq. Timothee Maret DataInputStream.readFully(byte[]) introduces no platform dependency since this just makes sure all bytes are read as-is.

Indeed, DataInputStream representation should be compatible among platforms.. but AFAIK, it is an opaque binary format.
If the Sling.id.file is not meant to be edited, then whatever representation would fit.
The DataInputStream has the advantage of not requiring an ad-hoc reader given the no external lib constraint.

With the DataInputStream solution, I think it would make sense to offer a way to allow users to specify sling identifiers.
This would allow to define structured scheme of identifiers which would be easier for a human to deal with.


was (Author: marett):
bq. Timothee Maret DataInputStream.readFully(byte[]) introduces no platform dependency since this just makes sure all bytes are read as-is.

Indeed, DataInputStream representation should be compatible among platforms.. but AFAIK, it is an opaque binary format.
If the Sling.id.file is not meant to be edited, then whatever representation would fit.
The DataInputStream has the advantage of not requiring an ad-hoc reader given the no external lib constraint.

In any case, I think it would make sense to offer a way to allow users to specify sling identifiers.
This would allow to define structured scheme of identifiers which would be easier for a human to deal with.

> Instance Sling Identifier may be randomly reset on restart
> ----------------------------------------------------------
>
>                 Key: SLING-3737
>                 URL: https://issues.apache.org/jira/browse/SLING-3737
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>    Affects Versions: Settings 1.3.0
>            Reporter: Timothee Maret
>            Assignee: Carsten Ziegeler
>             Fix For: Settings 1.3.2
>
>         Attachments: SLING-3737.diff, SLING-3737.diff
>
>
> In our Setup, we experience instances changing their Sling identifier upon restart.
> We have experienced only a few occurrences of this issue, but the effect is really bad, turning the services relying on the Sling Identifier into unexpected state (for instance Sling discovery service).
> We have checked that the sling.id.file was present before the issue occurred.
> We also checked that the value in this file was valid (36 byte long UUID).
> Despite having a valid sling.id.file file, the instance sometimes reset the Sling ID upon restart.
> Looking at the latest code in org.apache.sling.settings.impl.SlingSettingsServiceImpl#readSlingId
> it seems there is a bug in the way the sling id is read from the file sling.id.file.
> {code}
> private String readSlingId(final File idFile) {
>         if (idFile.exists() && idFile.length() >= 36) {
>             FileInputStream fin = null;
>             try {
>                 fin = new FileInputStream(idFile);
>                 final byte[] rawBytes = new byte[36];
>                 if (fin.read(rawBytes) == 36) {
>                     final String rawString = new String(rawBytes, "ISO-8859-1");
>                     // roundtrip to ensure correct format of UUID value
>                     final String id = UUID.fromString(rawString).toString();
>                     logger.debug("Got Sling ID {} from file {}", id, idFile);
>                     return id;
>                 }
>             } catch (final Throwable t) {
>                 logger.error("Failed reading UUID from id file " + idFile
>                         + ", creating new id", t);
>             } finally {
>                 if (fin != null) {
>                     try {
>                         fin.close();
>                     } catch (IOException ignore) {
>                     }
>                 }
>             }
>         }
>         return null;
>     }
> {code}
> In the line
> {code}
> if (fin.read(rawBytes) == 36) {
> {code}
> The code miss uses the java.io.FileInputStream#read API.
> {code}
> /**
>      * Reads up to <code>b.length</code> bytes of data from this input
>      * stream into an array of bytes. This method blocks until some input
>      * is available.
>      *
>      * @param      b   the buffer into which the data is read.
>      * @return     the total number of bytes read into the buffer, or
>      *             <code>-1</code> if there is no more data because the end of
>      *             the file has been reached.
>      * @exception  IOException  if an I/O error occurs.
>      */
> {code}
> The API stipulates that the method blocks until if finds *some* data.
> This is a common pattern with Java IO APIs, and indeed, the method may return with only one byte read even though the end of the stream was not reached.
> If this is the case, the current logic will treat the slingId as invalid and generate a new one.
> A way to fix that is to read the sling id file completely, for instance using the org.apache.commons.io.FileUtils API
> We are running on CentOS 6.2, JDK 1.7.



--
This message was sent by Atlassian JIRA
(v6.2#6252)