Suppress hdfsFixture if there are spaces in the path

[DaveCTurner]

Tests such as https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+6.x+multijob-unix-compatibility/os=ubuntu%20&&%20virtual/1015/console are failing because their path contains spaces, which causes issues with hdfsFixture. This change skips these tests in this situation.

05:21:07     2018-05-01 05:20:52,697 WARN  [DataNode: [[[DISK]file:/var/lib/jenkins/workspace/elastic+elasticsearch+6.x+multijob-unix-compatibility/os/ubuntu%2520&&%2520virtual/plugins/repository-hdfs/build/fixtures/hdfsFixture/hdfs-data/data/data1/, [DISK]file:/var/lib/jenkins/workspace/elastic+elasticsearch+6.x+multijob-unix-compatibility/os/ubuntu%2520&&%2520virtual/plugins/repository-hdfs/build/fixtures/hdfsFixture/hdfs-data/data/data2/]]  heartbeating to localhost/127.0.0.1:9999] datanode.DataNode (BPServiceActor.java:run(779)) - Unexpected exception in block pool Block pool <registering> (Datanode Uuid 34160522-c1dc-4082-94c1-5771444285ea) service to localhost/127.0.0.1:9999
05:21:07     java.util.UnknownFormatConversionException: Conversion = '2'
05:21:07     	at java.util.Formatter.checkText(Formatter.java:2579)
05:21:07     	at java.util.Formatter.parse(Formatter.java:2555)
05:21:07     	at java.util.Formatter.format(Formatter.java:2501)
05:21:07     	at java.util.Formatter.format(Formatter.java:2455)
05:21:07     	at java.lang.String.format(String.java:2940)
05:21:07     	at com.google.common.util.concurrent.ThreadFactoryBuilder.setNameFormat(ThreadFactoryBuilder.java:71)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsVolumeImpl.initializeCacheExecutor(FsVolumeImpl.java:153)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsVolumeImpl.<init>(FsVolumeImpl.java:135)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsDatasetImpl.addVolume(FsDatasetImpl.java:436)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsDatasetImpl.<init>(FsDatasetImpl.java:333)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsDatasetFactory.newInstance(FsDatasetFactory.java:34)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.FsDatasetFactory.newInstance(FsDatasetFactory.java:30)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.DataNode.initStorage(DataNode.java:1579)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.DataNode.initBlockPool(DataNode.java:1527)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.BPOfferService.verifyAndSetNamespaceInfo(BPOfferService.java:327)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.BPServiceActor.connectToNNAndHandshake(BPServiceActor.java:266)
05:21:07     	at org.apache.hadoop.hdfs.server.datanode.BPServiceActor.run(BPServiceActor.java:746)
05:21:07     	at java.lang.Thread.run(Thread.java:748)

[elasticmachine]

Pinging @elastic/es-core-infra

[jbaiera]

Left some comments. On second thinking, is that path already HTTP encoded when gradle is executing, or is it encoded later than this check?

[jbaiera]

I'm not sure a regular space in the path is what was causing the original issue. The issue was because a space character was HTTP encoded to %20 (and then encoded again to %2520). The % sign with numbers trips up string formatting in Java, which is what caused the exception.

Perhaps it might make sense to try formatting the string the same way HDFS does in a try block, catch the exception and mark the path illegal in that case.

[jbaiera]

The offending code in HDFS does:

ThreadFactory workerFactory = new ThreadFactoryBuilder()
        .setDaemon(true)
        .setNameFormat("FsVolumeImplWorker-" + parent.toString() + "-%d") // <----
        .build();

Which checks the name format eagerly to make sure it works with:

public ThreadFactoryBuilder setNameFormat(String nameFormat) {
    String.format(nameFormat, 0); // fail fast if the format is bad or null
    this.nameFormat = nameFormat;
    return this;
  }

So perhaps we could try formatting the string the exact same way HDFS does to ensure max compatibility in this regard.

try {
    // See o.a.h.h.s.d.f.i.FsVolumeImpl#initializeCacheExecutor:150
    String.format(rootProject.rootDir.toString() + '-%d', 0) 
} catch (IllegalFormatException ife) {
    legalPath = false;
}

[DaveCTurner]

Ok, the URL encoding hasn't happened by this point, so looking for a literal space finds the issue here:

$ pwd 
/var/lib/jenkins/workspace/elastic+elasticsearch+6.x+multijob-unix-compatibility/os/ubuntu && virtual
$ pwd | xxd
00000000: 2f76 6172 2f6c 6962 2f6a 656e 6b69 6e73  /var/lib/jenkins
00000010: 2f77 6f72 6b73 7061 6365 2f65 6c61 7374  /workspace/elast
00000020: 6963 2b65 6c61 7374 6963 7365 6172 6368  ic+elasticsearch
00000030: 2b36 2e78 2b6d 756c 7469 6a6f 622d 756e  +6.x+multijob-un
00000040: 6978 2d63 6f6d 7061 7469 6269 6c69 7479  ix-compatibility
00000050: 2f6f 732f 7562 756e 7475 2026 2620 7669  /os/ubuntu && vi
00000060: 7274 7561 6c0a                           rtual.

However, from the code you've shown it looks like any path that contains anything that needs URL-encoding is going to cause issues here - % for instance - so I agree that trying to catch an IllegalFormatException seems like it'll be more faithful. Can you dig out how the URL-encoding is happening (e.g. does it use java.net.URLEncoder (twice?!) or is it more complicated)?

[jbaiera]

That's a good question. I'll take a look but I wouldn't be surprised if its something deep in Hadoop or far away from where it's breaking. If it's not encoded at this point the space check should be fine enough then. Thanks!

[jbaiera]

The path is not encoded at the time of the build script running. This LGTM.

[jasontedor]

LGTM.

[DaveCTurner]

Marking this as backport pending to 6.3 as a naive backport fails for me and it'll take some time to dig into why.