ui: fix default jobs page sort #30423
Merged
Conversation
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
In cockroachdb#18137 a column was added, but the default sort (which is specified by index) was not updated. This updates it to correctly sort on creation time. Fixes: cockroachdb#30418 Release note (admin ui change): Fixes an issue where the default sort on the Jobs page was incorrectly the User column rather than the Creation Time.
|
This change is |
|
@piyush-singh I think this is worth backporting to 2.0, do you agree? |
|
Yes, having the ordering change frequently would be terribly confusing, and this is a relatively small change. |
|
Huh, I thought I had broken this in my jobs page update PR, and fixed it there as well. Good catch. |
vilterp
approved these changes
Sep 19, 2018
|
bors r+ |
Build failed (retrying...) |
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Sep 19, 2018
30394: sqlbase: bugfix in rowfetcher with multiple cfs r=jordanlewis a=jordanlewis A bug (introduced during the 2.1 cycle) caused scans issued via distsql against tables with multiple column families to return incorrect results or panic the server if some of the column families were marked as non-nullable. This bug could only occur in distsql because it only manifested when a rowfetcher was initialized with multiple input spans, which only occurs in distsql where a particular node owns 2 non-contiguous portions of the keyspace. In this case, the code would misinterpret the empty response from the scan request on the second input span to mean that there was a limit on the last scan and that we might have entered a new input span, which caused the rowfetcher to mark the current row as ended and expect a full row starting from the next key retrieved. This was not the case, as this could happen in normal circumstances. When a row had multiple column families, the rowfetcher would fail to remember the results it had already seen for the earlier column families in a row, if a batch happened to end half way through the row's column families, and return incorrect and/or panic depending on the definition of the table. This patch removes this problem and adds a test for it. The test is a little complicated but I verified that it does in fact panic before the patch. It should catch problems like this in the future. We had no explicit tests of the rowfetcher before with multiple input spans. Note that this patch also deletes a test case, that tests correct behavior in the case of seeing duplicate rows from the input stream during an index join. This test case is contrived - index joins can never encounter duplicate rows from the input stream - so it's safe to delete. Closes #29374. Partially reverts #23474. The code no longer has protection against the condition that that PR was trying to guard against. But I don't think this is a problem in practice. If it crops up in new code it'll be very easy to notice and fix at that point. cc @vivekmenezes Release note: None 30423: ui: fix default jobs page sort r=couchand a=couchand In #18137 a column was added, but the default sort (which is specified by index) was not updated. This updates it to correctly sort on creation time. Fixes: #30418 Release note (admin ui change): Fixes an issue where the default sort on the Jobs page was incorrectly the User column rather than the Creation Time. Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com> Co-authored-by: Andrew Couch <andrew@cockroachlabs.com>
Build succeeded |
This was referenced Sep 19, 2018
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.
In #18137 a column was added, but the default sort
(which is specified by index) was not updated. This
updates it to correctly sort on creation time.
Fixes: #30418
Release note (admin ui change): Fixes an issue where
the default sort on the Jobs page was incorrectly
the User column rather than the Creation Time.