Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client connections resolve DNS asynchronously #3140

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

jumaffre
Copy link
Contributor

@jumaffre jumaffre commented Oct 27, 2021

DNS resolution was made synchronous in #70 for listening sockets, but isn't necessary for client connections, which could stall the host thread if the DNS resolution takes too long, causing spurious elections.

FYI, using the socket_getaddrinfo CLI, I've managed to hit ~4sec stall for DNS resolution in rare cases, running

$ for ITER in {1..1000}; do time socket_getaddrinfo invaliddomain${ITER}.invaliddomain${ITER}.invaliddomain${ITER} 12345; done
...
Cannot getaddrinfo() - Name or service not known

real    0m3.976s
user    0m0.012s
sys     0m0.004s
...

@ghost
Copy link

ghost commented Oct 27, 2021

async_dns_resolution_client@35292 aka 20211027.15 vs main ewma over 20 builds from 34910 to 35269

Click to see table
build_id build_number tpcc_sgx_cft^ tpcc_sgx_cft_mem ls_sgx_cft^ ls_sgx_cft_mem ls_jwt_sgx_cft^ ls_jwt_sgx_cft_mem ls_js_sgx_cft^ ls_js_sgx_cft_mem ls_full_js_sgx_cft^ ls_full_js_sgx_cft_mem ls_js_jwt_sgx_cft^ ls_js_jwt_sgx_cft_mem Historical query (/s)^ CHAMP put (/s)^ CHAMP get (/s)^
34910 20211021.14 6638.7 9.03128e+07 24054 1.82232e+07 4492.71 1.56017e+07 2675.69 1.0621e+07 2385.34 1.40289e+07 1811.96 7.99955e+06 8394.45 1.45693e+06 3.55556e+07
34932 20211022.2 6740.2 9.21478e+07 24350.6 1.7961e+07 4562.06 1.56017e+07 2721.85 1.00967e+07 2376.15 1.03588e+07 1810.07 8.52384e+06 8195.98 1.42896e+06 3.58669e+07
34972 20211022.14 6545.38 9.13613e+07 24492.2 1.7961e+07 4556.96 1.56017e+07 2724.6 1.0621e+07 2455.65 1.2456e+07 1827.89 8.52384e+06 7847.54 1.45206e+06 3.55556e+07
35003 20211022.23 6544.74 9.16235e+07 24300.1 1.7961e+07 4551.02 1.56017e+07 2709 1.03588e+07 2378.04 1.40289e+07 1821.85 8.52384e+06 7956.33 1.48372e+06 3.57411e+07
35016 20211022.27 6547.07 9.13613e+07 24071.2 1.76989e+07 4524.02 1.56017e+07 2710.43 1.29803e+07 2379.48 1.35046e+07 1785.36 1.03588e+07 7554.74 1.45858e+06 3.59298e+07
35053 20211023.1 6595.38 9.13613e+07 24352.5 1.82232e+07 4527 1.58639e+07 2738.27 1.40289e+07 2417.84 1.37667e+07 1816.3 8.78598e+06 8322.63 1.45899e+06 3.58042e+07
35060 20211023.3 6775.95 9.08371e+07 24425.5 1.76989e+07 4535.04 1.53396e+07 2639.46 1.0621e+07 2445.71 1.03588e+07 1799.51 8.52384e+06 8357.06 1.46055e+06 3.56794e+07
35071 20211025.2 6704.11 9.16235e+07 24498.9 1.82232e+07 4507.38 1.53396e+07 2746.8 1.00967e+07 2455.35 1.03588e+07 1784.15 8.52384e+06 8595.11 1.45309e+06 3.56788e+07
35086 20211025.7 6519.56 9.13613e+07 24378 1.76989e+07 4433.86 1.56017e+07 2745.64 1.0621e+07 2407.49 1.32424e+07 1832.2 8.2617e+06 8170.43 1.45237e+06 3.56788e+07
35097 20211025.10 6381.6 8.97885e+07 24256.4 1.82232e+07 4488.96 1.58639e+07 2759.79 1.32424e+07 2419 1.03588e+07 1793.24 8.2617e+06 8196.02 1.47837e+06 3.56794e+07
35132 20211026.2 6763.62 9.16235e+07 24423.5 1.84853e+07 4531.08 1.53396e+07 2707.85 1.35046e+07 2436.92 1.03588e+07 1829.39 8.2617e+06 8184.4 1.47635e+06 3.64413e+07
35137 20211026.4 6788.48 9.26721e+07 24270.4 1.76989e+07 4421.64 1.56017e+07 2753.42 1.4291e+07 2437.96 1.03588e+07 1803.46 8.2617e+06 9260.95 1.48631e+06 3.61831e+07
35153 20211026.8 6615.95 9.10992e+07 24364.6 1.69124e+07 4361.54 1.53396e+07 2743.92 1.37667e+07 2428.63 1.03588e+07 1809.96 8.52384e+06 7769.25 1.46076e+06 3.63121e+07
35168 20211026.12 6214.97 9.08371e+07 23214.8 1.66503e+07 4259.89 1.53396e+07 2740.33 1.03588e+07 2435.48 1.00967e+07 1797.92 8.52384e+06 8395.47 1.46358e+06 3.58036e+07
35189 20211026.19 6512.18 9.10992e+07 24353.6 1.71746e+07 4237.04 1.58639e+07 2738.87 1.4291e+07 2422.56 1.03588e+07 1816.66 8.52384e+06 8155.7 1.47965e+06 3.58669e+07
35220 20211026.29 6661.45 9.13613e+07 23234.2 1.7961e+07 4366.58 1.53396e+07 2686.06 1.03588e+07 2386.23 1.00967e+07 1808.19 8.52384e+06 8457.82 1.4501e+06 3.55549e+07
35236 20211026.34 6773.17 9.08371e+07 24510.8 1.66503e+07 4401.08 1.56017e+07 2742.66 1.32424e+07 2435.79 1.00967e+07 1808.66 8.52384e+06 8495.54 1.45952e+06 3.58663e+07
35249 20211027.2 6747.97 9.18856e+07 24075.5 1.69124e+07 4306.71 1.56017e+07 2687.13 1.0621e+07 2393.83 1.00967e+07 1798.58 8.52384e+06 8475.09 1.45454e+06 3.65714e+07
35259 20211027.6 6670 9.18856e+07 24261.9 1.76989e+07 4416.97 1.56017e+07 2751.38 1.03588e+07 2383.15 1.00967e+07 1782.84 8.52384e+06 8325.84 1.46988e+06 3.63759e+07
35269 20211027.9 6590.97 9.13613e+07 23550.9 1.66503e+07 4326.76 1.50774e+07 2706.2 1.00967e+07 2388.93 1.29803e+07 1776 8.52384e+06 8591.05 1.45599e+06 3.59298e+07

images

@jumaffre jumaffre marked this pull request as ready for review October 27, 2021 12:13
@jumaffre jumaffre requested a review from a team October 27, 2021 12:13
@achamayou
Copy link
Member

@jumaffre isn't this going to bring back the problem that #70 was fixing? Don't we need an additional wait to prevent that?

@jumaffre
Copy link
Contributor Author

@jumaffre isn't this going to bring back the problem that #70 was fixing? Don't we need an additional wait to prevent that?

@achamayou I don't believe so. The issue described in #70 is only about the listening sockets on node start-up (which are still synchronous in this PR). This PR only changes the behaviour for client connections.

@achamayou
Copy link
Member

@jumaffre indeed, thank you for pointing this out!

@jumaffre jumaffre merged commit 6834bf9 into microsoft:main Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants