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

Add repair-brig-clients-table to clean up after the fix in #3504. #3507

Merged
merged 9 commits into from
Aug 17, 2023

Conversation

fisx
Copy link
Contributor

@fisx fisx commented Aug 16, 2023

https://wearezeta.atlassian.net/browse/WPB-3888

make c package=repair-brig-clients-table
./dist/repair-brig-clients-table --dry-run --cassandra-port-brig 9044 --cassandra-keyspace-brig brig > repair-brig-clients-table.log 2>&1

Without --dry-run this will actually delete the offending entries.

I will attach the log as soon as it's done (currently at 7M clients and 30 of them having all fields NULL except keys and last_active).

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Aug 16, 2023
@fisx fisx marked this pull request as ready for review August 16, 2023 15:53
@fisx
Copy link
Contributor Author

fisx commented Aug 16, 2023

$ cat repair-brig-clients-table.log | grep '\*\*\*' | wc
     36     360    6323

$ cat repair-brig-clients-table.log | wc
   9013   54166  375779

so, 36 broken clients. i suggest we remove them?

@fisx
Copy link
Contributor Author

fisx commented Aug 16, 2023

I think this is ready to merge (whether integration tests have passed or not...)

@@ -0,0 +1,94 @@
{-# LANGUAGE OverloadedStrings #-}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a default plugin, as defined in the cabal file? 🤔

@@ -0,0 +1,93 @@
{-# LANGUAGE OverloadedStrings #-}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default extension as defined in the cabal file, isn't it?

@@ -0,0 +1,93 @@
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE ScopedTypeVariables #-}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default extension as defined in the cabal file, isn't it?

@@ -0,0 +1,93 @@
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# OPTIONS_GHC -Wno-orphans -Wno-unused-imports #-}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these options needed here at all? 🤔

@fisx fisx requested a review from supersven August 17, 2023 10:03
Copy link
Contributor

@supersven supersven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@fisx
Copy link
Contributor Author

fisx commented Aug 17, 2023

CI failure is unrelated.

@fisx fisx merged commit 48ac13c into develop Aug 17, 2023
7 of 9 checks passed
@fisx fisx deleted the WPB-3888-repair-db branch August 17, 2023 10:48
supersven added a commit that referenced this pull request Oct 5, 2023
…3507)

Co-authored-by: Sven Tennie <sven.tennie@wire.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants