-
Notifications
You must be signed in to change notification settings - Fork 26
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
DNS lookup of EET endpoints limited by timeout (default 2s)
- Loading branch information
Showing
10 changed files
with
288 additions
and
28 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
34 changes: 34 additions & 0 deletions
34
src/main/java/cz/tomasdvorak/eet/client/concurency/CompletedFuture.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package cz.tomasdvorak.eet.client.concurency; | ||
|
||
import java.util.concurrent.ExecutionException; | ||
import java.util.concurrent.Future; | ||
import java.util.concurrent.TimeUnit; | ||
import java.util.concurrent.TimeoutException; | ||
|
||
public class CompletedFuture<T> implements Future<T> { | ||
|
||
@Override | ||
public boolean cancel(final boolean mayInterruptIfRunning) { | ||
return false; | ||
} | ||
|
||
@Override | ||
public boolean isCancelled() { | ||
return false; | ||
} | ||
|
||
@Override | ||
public boolean isDone() { | ||
return false; | ||
} | ||
|
||
@Override | ||
public T get() throws InterruptedException, ExecutionException { | ||
return null; | ||
} | ||
|
||
@Override | ||
public T get(final long timeout, final TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { | ||
return null; | ||
} | ||
} |
26 changes: 24 additions & 2 deletions
26
src/main/java/cz/tomasdvorak/eet/client/dto/WebserviceConfiguration.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,42 @@ | ||
package cz.tomasdvorak.eet.client.dto; | ||
|
||
/** | ||
* TODO: create builder for the configuration! | ||
*/ | ||
public class WebserviceConfiguration { | ||
|
||
|
||
public static final long RECEIVE_TIMEOUT = 2000L; | ||
public static final long DNS_LOOKUP_TIMEOUT = 2000L; | ||
|
||
public static final WebserviceConfiguration DEFAULT = new WebserviceConfiguration( | ||
2000L // timeout for webservice call in millis = 2s, required by the current laws | ||
RECEIVE_TIMEOUT, // timeout for webservice call in millis = 2s, required by the current laws | ||
DNS_LOOKUP_TIMEOUT // timeout for DNS lookup in millis = 2s | ||
); | ||
private final long receiveTimeout; | ||
private long receiveTimeout; | ||
private long dnsLookupTimeout; | ||
|
||
/** | ||
* @param receiveTimeout receiving timeout of the Webservice call in millis | ||
*/ | ||
public WebserviceConfiguration(final long receiveTimeout) { | ||
this(receiveTimeout, DNS_LOOKUP_TIMEOUT); | ||
} | ||
|
||
/** | ||
* @param receiveTimeout receiving timeout of the Webservice call in millis | ||
* @param dnsLookupTimeout timeout for DNS lookup in millis | ||
*/ | ||
public WebserviceConfiguration(final long receiveTimeout, final long dnsLookupTimeout) { | ||
this.receiveTimeout = receiveTimeout; | ||
this.dnsLookupTimeout = dnsLookupTimeout; | ||
} | ||
|
||
public long getReceiveTimeout() { | ||
return receiveTimeout; | ||
} | ||
|
||
public long getDnsLookupTimeout() { | ||
return dnsLookupTimeout; | ||
} | ||
} |
10 changes: 10 additions & 0 deletions
10
src/main/java/cz/tomasdvorak/eet/client/exceptions/DnsLookupFailedException.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package cz.tomasdvorak.eet.client.exceptions; | ||
|
||
/** | ||
* DNS lookup failed from some other reason than timeout. It may be UnknownHostException, MalformedURLException or similar. | ||
*/ | ||
public class DnsLookupFailedException extends Exception { | ||
public DnsLookupFailedException(final String message, final Throwable cause) { | ||
super(message, cause); | ||
} | ||
} |
10 changes: 10 additions & 0 deletions
10
src/main/java/cz/tomasdvorak/eet/client/exceptions/DnsTimeoutException.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package cz.tomasdvorak.eet.client.exceptions; | ||
|
||
/** | ||
* DNS lookup failed after timeout reached. | ||
*/ | ||
public class DnsTimeoutException extends Exception { | ||
public DnsTimeoutException(final String message, final Throwable cause) { | ||
super(message, cause); | ||
} | ||
} |
8 changes: 8 additions & 0 deletions
8
src/main/java/cz/tomasdvorak/eet/client/networking/DnsResolver.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package cz.tomasdvorak.eet.client.networking; | ||
|
||
import cz.tomasdvorak.eet.client.exceptions.DnsLookupFailedException; | ||
import cz.tomasdvorak.eet.client.exceptions.DnsTimeoutException; | ||
|
||
public interface DnsResolver { | ||
String resolveAddress(String url) throws DnsLookupFailedException, DnsTimeoutException; | ||
} |
63 changes: 63 additions & 0 deletions
63
src/main/java/cz/tomasdvorak/eet/client/networking/DnsResolverWithTimeout.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
package cz.tomasdvorak.eet.client.networking; | ||
|
||
import cz.tomasdvorak.eet.client.exceptions.DnsLookupFailedException; | ||
import cz.tomasdvorak.eet.client.exceptions.DnsTimeoutException; | ||
import cz.tomasdvorak.eet.client.security.SecureEETCommunication; | ||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
|
||
import java.net.InetAddress; | ||
import java.net.MalformedURLException; | ||
import java.net.URL; | ||
import java.net.UnknownHostException; | ||
import java.util.concurrent.*; | ||
|
||
public class DnsResolverWithTimeout implements DnsResolver { | ||
|
||
private static final Logger logger = LogManager.getLogger(SecureEETCommunication.class); | ||
|
||
private final long timeoutMillis; | ||
|
||
public DnsResolverWithTimeout(long timeoutMillis) { | ||
this.timeoutMillis = timeoutMillis; | ||
} | ||
|
||
@Override | ||
public String resolveAddress(final String url) throws DnsLookupFailedException, DnsTimeoutException { | ||
final String host; | ||
try { | ||
host = new URL(url).getHost(); | ||
} catch (MalformedURLException e) { | ||
throw new DnsLookupFailedException(String.format("URL %s is malformed", url), e); | ||
} | ||
|
||
ExecutorService executor = Executors.newSingleThreadExecutor(); | ||
try { | ||
Future<String> result = executor.submit(new Callable<String>() { | ||
@Override | ||
public String call() throws Exception { | ||
return doResolve(host); | ||
} | ||
}); | ||
return result.get(timeoutMillis, TimeUnit.MILLISECONDS); | ||
} catch (InterruptedException e) { | ||
logger.warn("Unexpected interrupton while resolving host " + host, e); | ||
Thread.currentThread().interrupt(); | ||
throw new DnsLookupFailedException("Unexpected interruption while resolving host " + host, e); | ||
} catch (ExecutionException e) { | ||
throw new DnsLookupFailedException("Failed resolving host " + host, e.getCause()); | ||
} catch (TimeoutException e) { | ||
throw new DnsTimeoutException(String.format("DNS Lookup for host %s timed out", host), e); | ||
} finally { | ||
executor.shutdownNow(); | ||
} | ||
} | ||
|
||
/** | ||
* Internal method to allow easier unit testing without dependency on internet connection | ||
*/ | ||
protected String doResolve(final String host) throws UnknownHostException { | ||
InetAddress address = InetAddress.getByName(host); | ||
return address.getHostAddress(); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mohu vedet jaky je cil teto upravy?
Nasadil jsem dnes tuto verzi s 3s timoutem pro lookup a dnes kolem 22h vubec prvni timeout(dns) za asi tyden provozu. Diky
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Java nedovede nijak nastavit timeout pro DNS lookup. Pokud je tedy síť nebo DNS v nějakém nefunkčním stavu, může lookup trvat běžně 20s, než se operační systém rozhodne to vzdát. To má za následek, že odesílání dat může trvat mnoho desítek sekund, než skončí chybou (UnknownHostException obalená do CommunicationException).
Takové chování je velmi nepříjmené pro toho, kdo pošle data skrze knihovnu k EET a očekává rychlou odpověď, aby mohl zpracovat účtenku. Navenek se více jak 20s nic neděje a pak odeslání končí chybou.
Je tedy nutné řešit timeout pro DNS lookup explicitně ve vlastním vlákně, aby bylo možné včas celou operaci zastavit a vyhodit správnou výjimku (CommunicationTimeoutException).
Pokud vám operace vytimeoutovala, pak bylo zřejmě připojení k internetu nebo DNS serveru dočasně nefunkční.
Je z vašeho pohledu onen zaznamenaný timeout v pořádku?
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zda se ze to timeoutuje docela casto na tom DNS. Dnes dalsi v jine siti.
Ja to v principu chapu, ale pokud pred kazdym hlasem testuji preklad nazvu domeny a pak nasledne tu domenu pouziju ve volani te servisy, tak mi nic nezaruci ze ten preklad probehne znova v samotnem volani metody ws. Nemela by to nejak resit spise implementace tech apache ws knihoven?
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ten test využívá třídu InetAddress z javy. Ta má v základu zabudovanou cache: https://docs.oracle.com/javase/7/docs/api/java/net/InetAddress.html . Pokud tedy lookup proběhne v pořádku, v samotném volání se použije již resolvovaná adresa, stejně tak jako při dalším volání v rámci instance JVM. Pokud lookup selže, selže (včas) i celé volání EET.
Já resolvovanou adresu nijak dál explicitně nepředávám a neměním tak nijak chování, které bylo nastavené až dosud. Pouze provedu lookup z kontrolovatelného a timeoutovaného vlákna předem.
Implementace apache WS knihoven to nijak neřeší. Java pro to nemá žádný mechanismus ani konfigurovatelný timeout a je zcela závislá na chování operačního systému. Proto je třeba vyřešit to manuálně předem. Ty apache knihovny pak už jen znovu využívají třídu InetAddress a získají tak adresu z cache.
Pokud jsem něco nepřehlédl, nemělo by to, krom samotné obsluhy vlákna, představovat žádný overhead ani duplicitní lookup.
Tady ještě link na stejnou issue v dotnet implementaci: https://github.com/MewsSystems/eet/issues/10
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pak me teda zarazi relativne caste timeouty, zvlast kdyz pouzivame lokalni DNS a zaden vypadek neni. Zda se teda ze ty 3s jsou v realu nejspie malo. Mozna bych se primlouval moznosti tuto funkci vypnout napr nastavenim hodnoty timeoutu na 0ms, to by znamenalo minimalni upravu...
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jinak jeste ten timeout by mel jit i nastavit v systemu, konkretne v linuxu v /etc/resolv.conf
options timeout:2
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disable té funkcionality je dobrý nápad, doplním v příští verzi (případně rád přijmu pull request). V tuto chvíli můžete nastavit nějaký poměrně vysoký timeout (20s?) a mělo by vše fungovat.
Jinak by mě zajímalo, jestli opravdu dns lookup trvá tak dlouho a vlákno vytiká oprávněně. Dovedl byste nějak ověřit, jak se to ve vaší síti chová?
Timeout můžete v systému ovlivnit, ale velká část uživatelů to nedovede / nemá možnost změnit. Pro ně bohužel takové řešení nepřipadá v úvahu.
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevim jak bych to overil, to se bude tezko ladit takovato vec. Jestli to nevytika treba na nejake rezii java VM, pri tvorbe vlaken nebo treba se v tu chvili pusti GC.
0464876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zkusim to jeste s limitem 5s. ALe v mem pripade je tato funcionalia asi zbytecna, nebot to bezi jako webova servisa a pokud bude sitovy problem, tak se klient k te sluzbe vubec neprihlasi a tim padem je tento krok imho zbytecny a nepredpoklada se, ze se nejak zmeni nastaveni lokalniho resolveru. Pull req jsem jiz zaslal. Diky