codewars / codewars.com

Issue tracker for Codewars
https://www.codewars.com
BSD 2-Clause "Simplified" License
2.09k stars 219 forks source link

Race conditions in translation approvals #1189

Open Voileexperiments opened 6 years ago

Voileexperiments commented 6 years ago

This has occurred to this translation of this kata:

Translation approvals does not seem to handle race conditions properly. If two separate users approve the same translation at once, it'll end up being approved twice and now we have 2 versions of the same language.

┆Issue is synchronized with this Clickup by Unito

Voileexperiments commented 6 years ago

Related: #1009

Voileexperiments commented 6 years ago

This is happening again:

https://www.codewars.com/kata/scytale-encoder-slash-decoder-ancient-spartans-cipher

There are 2 JS language versions. Incidentally, it also causes JS version edits to fail to be applied.

hobovsky commented 5 years ago

Two C versions: https://www.codewars.com/kata/square-matrix-multiplication

10XL commented 5 years ago

2 Ruby versions https://www.codewars.com/kata/split-and-then-add-both-sides-of-an-array-together

Voileexperiments commented 5 years ago

Is this related to #1683?

kazk commented 5 years ago

Not related to #1683, but the same idea. Bugs from race conditions like this unavoidable unless the data integrity is enforced at the database level.

1683 shouldn't happen if proper unique index was there (user will always have one document per kata because write will fail on attempt to add a document that violates the condition). To be honest, I'm not sure why that's happening to begin with because that should only involve one user. But the ORM is making implicit database calls all the time so it's probably just something hidden.

This one is embedded document so unique index (within the document embedded in) is not even supported. So I'm not really sure what to do.

MongoDB is obviously not great for relational data and it's very difficult (or just needs more work) to maintain data integrity. I wish Codewars used PostgreSQL, but it is what it is.

Voileexperiments commented 5 years ago

If kata data is structured like what I can see from the editor publish requests I think you can do this, just need to add an extra key to all the available languages as strings per kata.

kazk commented 5 years ago

@Voileexperiments thanks, yeah, something like that should minimize the risk.

Languages are embedded in kata and it has "name" key.

{
  name: "Multiply",
  languages: [
    {
      name: "javascript"
    }
  ]
}

The ORM should be validating the uniqueness of languages.name locally (so adding another javascript to above will fail), but that's the best they can do.

When a translation is approved we currently do the following to merge a language (after checking description conflict etc).

  1. get the embedded document by language name or initialize one in memory if not found
  2. copy the data from translation into this document
  3. save the kata document
hobovsky commented 5 years ago

Two Java versions: https://www.codewars.com/kata/path-finder-number-4-where-are-you

suic86 commented 5 years ago

Coffeescript double translation

javatlacati commented 5 years ago

Same thing hapenes to this Kata:

image

Steffan153 commented 5 years ago

If we're supposed to post them here, there's a bunch:

I see them all the time. Quite a bug!

Steffan153 commented 5 years ago

Hmmm..... found some more O_o:

Steffan153 commented 4 years ago

Another one: https://www.codewars.com/kata/567e8f7b4096f2b4b1000005/csharp

Blind4Basics commented 4 years ago

seems it happened again there with JS : https://www.codewars.com/kata/5f55ecd770692e001484af7d

hobovsky commented 1 year ago

Two C++ translations on this kata: https://www.codewars.com/kata/5cb7baa989b1c50014a53333/discuss#64a83efddddc25116edfd0bd

hobovsky commented 12 months ago

Two Crystal translations: https://www.codewars.com/kata/5d49c93d089c6e000ff8428c/crystal

hobovsky commented 3 months ago

Two CoffeeScript translations: https://www.codewars.com/kata/5b180e9fedaa564a7000009a