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.

Sign up for GitHub

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

Jump to bottom

build/teamcity: nightly Pebble metamorphic runs failed to create GH issue on failure #81546

Closed
nicktrav opened this issue May 19, 2022 · 8 comments
Closed

build/teamcity: nightly Pebble metamorphic runs failed to create GH issue on failure #81546

nicktrav opened this issue May 19, 2022 · 8 comments
Assignees
@nicktrav @rickystewart
Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-dev-inf

Comments

@nicktrav
Copy link
Collaborator

nicktrav commented May 19, 2022 •
edited by cockroach-jira-scripts
Loading

We had four nightly metamorphic test runs fail since 5/1, none of which created a corresponding issue on the Pebble repo notifying us of the failure. Typically, we get an issue like this.

For now I've created four separate issues manually so we can investigate:

Jira issue: CRDB-15081
@nicktrav nicktrav added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-storage Relating to our storage engine (Pebble) on-disk storage. T-storage Storage Team labels May 19, 2022
@nicktrav nicktrav self-assigned this May 19, 2022
@rickystewart
Copy link
Collaborator

Looking into why no issues were posted.

@rickystewart rickystewart self-assigned this May 19, 2022
@rickystewart rickystewart added T-dev-inf and removed T-storage Storage Team labels May 19, 2022
@rickystewart
Copy link
Collaborator

The scripts only post issues if TC_BUILD_BRANCH is one of master, release-*, or provisional-*. Looking in the logs I see this:

[06:01:08 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Nightlies_Pebble_Metamorphic/5157803?buildTab=log&focusLine=15555&linesState=15555)  + [[ refs/heads/master == master ]]
[06:01:08 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Nightlies_Pebble_Metamorphic/5157803?buildTab=log&focusLine=15556&linesState=15556)  + [[ refs/heads/master == release-* ]]
[06:01:08 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Nightlies_Pebble_Metamorphic/5157803?buildTab=log&focusLine=15557&linesState=15557)  + [[ refs/heads/master == provisional_* ]]

So in other words the TC_BUILD_BRANCH is refs/heads/master, not master as expected. I think that is because up until today the trigger for these Pebble build configurations has been set to +:refs/heads/master, not +:<default> as with the other nightlies. I updated the trigger accordingly and will check tomorrow to see if TC_BUILD_BRANCH is updated accordingly.

@nicktrav
Copy link
Collaborator Author

Cool - thanks for digging into that @rickystewart!

Does the timeline line up with when this would have broken? It's hard to tell from the history, as these tests tend to fail infrequently, and some of the test history has since rolled. I believe the last time we got a hit was this one (cockroachdb/pebble#1499), from back in Feb.

We also have some other nightlies, so it would be good to ensure those are set up to post too:

  • Nightly Pebble Metamorphic (this one)
  • Nightly Pebble Metamorphic Race
  • Pebble Nightly YCSB
  • Pebble Nightly YCSB Race
  • Pebble Nightly Write Throughput

@bananabrick
Copy link
Contributor

Aah, I may have changed this when I was setting up other nightly tests: So in other words the TC_BUILD_BRANCH is refs/heads/master, not master as expected., and not forseen the consequence. If that is indeed the case, I apologize for creating the sudden influx of issues which need to be investigated.

@rickystewart
Copy link
Collaborator

I'm looking at the history of the build configuration and don't see anything that explains things breaking on or around 5/1. I also see @bananabrick's changes and I don't think they caused this, but I may be wrong.

We also have some other nightlies, so it would be good to ensure those are set up to post too:

I have already looked through all the Pebble build configurations and updated the triggers to match up to the Pebble metamorphic trigger.

rickystewart added a commit to rickystewart/cockroach that referenced this issue May 20, 2022
@rickystewart
ci: update how the tc_release_branch check is performed
2119d27 
How TeamCity sets the `TC_BUILD_BRANCH` environment variable is a bit
unpredictable. While we expect it to be a string like `master`, at times
(see cockroachdb#81546) this variable can be set to a ref like `refs/heads/master`.
This inconsistency means that we are sometimes not reporting issues to
GitHub as we should be. To be resilient to this I make sure we strip
off the `refs/heads/` prefix when performing the `tc_release_branch`
check.

Release note: None
@rickystewart rickystewart mentioned this issue May 20, 2022
Merged
@rickystewart
Copy link
Collaborator

That change did not appear to do the trick so I have #81580.

craig bot pushed a commit that referenced this issue May 20, 2022
@xinhaoz @rickystewart
Merge #80707 #81580
001e88e 
80707: server: correctly redact sql constants in ListSessions api r=matthewtodd a=xinhaoz

Previously, there was a bug in the redaction of SQL constants
for `VIEWACTIVITYREDACTED` users in the `ListSessions` api.
Firstly, the range loop used to set the sql query field
was manipulating the copy of the query object provided
by the range. Secondly, the previous logic when redacting
set the `Sql` and `LastActiveQuery` fields to empty strings.
Since these fields are read in  other parts of the codebase
(e.g. to fill out columns of virtual tables), we should just
set these fields to equal `SqlNoConstants` and
`LastActiveQueryNoConstants` respectively.

Release note (bug fix): constants in sql query fields are
correctly removed for VIEWACTIVITYREDACTED users

81580: ci: update how the `tc_release_branch` check is performed r=rail a=rickystewart

How TeamCity sets the `TC_BUILD_BRANCH` environment variable is a bit
unpredictable. While we expect it to be a string like `master`, at times
(see #81546) this variable can be set to a ref like `refs/heads/master`.
This inconsistency means that we are sometimes not reporting issues to
GitHub as we should be. To be resilient to this I make sure we strip
off the `refs/heads/` prefix when performing the `tc_release_branch`
check.

Release note: None

Co-authored-by: Xin Hao Zhang <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
@jlinder jlinder added sync-me and removed sync-me labels May 20, 2022
@rickystewart
Copy link
Collaborator

Looks like the change worked:

[07:02:19 ]+ branch=master
[07:02:19 ]+ [[ master == master ]]

We should see it post issues next time.

@nicktrav
Copy link
Collaborator Author

Thank you!

andrewbaptist pushed a commit to andrewbaptist/cockroach that referenced this issue May 25, 2022
@rickystewart @andrewbaptist
ci: update how the tc_release_branch check is performed
1a13f17 
How TeamCity sets the `TC_BUILD_BRANCH` environment variable is a bit
unpredictable. While we expect it to be a string like `master`, at times
(see cockroachdb#81546) this variable can be set to a ref like `refs/heads/master`.
This inconsistency means that we are sometimes not reporting issues to
GitHub as we should be. To be resilient to this I make sure we strip
off the `refs/heads/` prefix when performing the `tc_release_branch`
check.

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nicktrav nicktrav

@rickystewart rickystewart

Labels
A-storage Relating to our storage engine (Pebble) on-disk storage. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-dev-inf
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jlinder @nicktrav @rickystewart @bananabrick