sql: add a validation job status during schema validation #36032
Conversation
This file contains hidden or 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 change is |
dt
approved these changes
Mar 28, 2019
|
TFTR! |
|
bors r+ |
Build failed (retrying...) |
|
bors r+ |
Build failed |
Release note: None
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
May 7, 2019
36032: sql: add a validation job status during schema validation r=vivekmenezes a=vivekmenezes related to #32916 Release note: None 37331: util/json: escape non-finite number values when marshaling r=mjibson a=mjibson Since the resulting value is a string, not a number, it is unclear how to test the roundtrip-ness of this, since it depends on the receiving language. In JavaScript both of the following are true: `Infinity == "Infinity"` `"Infinity" == Infinity` This explains why JSON didn't need to define +inf, -inf or NaN is special values and instead leaves that up to the runtime. Thus for now we only test that the output of this is parseable, not that it is equal, to its input. This output matches postgres. Fixes #37318 Release note (bug fix): correctly escape non-finite JSON number values when marshaling to text. 37344: *: ensure cluster-less RPC-related tests validate cluster IDs r=knz a=knz Fixes #35412. When a test uses RPCs but does not set up a fully-fledged cluster, its RPC context operates "without a cluster ID" throughout the test. This creates potential for a flake if the test creates a server and subsequently stops the server and expects further RPCs to that server to fail: it is possible for a concurrent unrelated test to start a server using the same TCP port as the server in the first test, in which case the first test will then (unexpectedly, and incorrectly) succeed in connecting to a server itself did not start. This failure mode exists because without a valid cluster ID, the RPC context does not validate what it's talking to. The likelihood of TCP port reuse on a single server process is relatively low, but given the frequency and sheer amount of tests that start and stop servers, the probability we would observe such a flake “in the wild” was asymptotically approaching 1, and indeed #35412 has this scenario as likely cause. This patch attempts to mitigate this situation by revisiting all the tests that currently spawn a client RPC context without spawning a cluster, in the cases where the test also checks the availability/status/liveness of its server(s). On these tests, the patch sets the cluster ID to be a random non-zero value. Note that the previous fix to check expected node IDs is also a partial mitigation, however most tests use node IDs starting from 1 (with most of them using just 1, or 1 and 2), and so the probability that two unrelated tests running concurrently on the same machine would run two server instances with the same node ID remains high. A random cluster ID thus constitutes a more effective, definite mitigation. Release note: None Co-authored-by: Vivek Menezes <[email protected]> Co-authored-by: Matt Jibson <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
related to #32916
Release note: None