Giter VIP home page Giter VIP logo

Comments (9)

markphelps avatar markphelps commented on May 19, 2024 1

@derekperkins I plan to try and tackle this over the long weekend coming up

from flipt.

markphelps avatar markphelps commented on May 19, 2024 1

@sagikazarmark thanks for the suggestion! i'll take a look for future use

The squirrel library im using to construct the queries does a pretty good job of abstracting the db specific stuff, so it's mainly just handling the different error codes that are thrown from the different drivers when there are constraint violations/etc.

I have a branch where im abstracting the sql code to be able to handle the specific errors more cleanly depending on the driver being used. I hope to have this finished this weekend.. just trying to find the time to work on it!

from flipt.

derekperkins avatar derekperkins commented on May 19, 2024 1

The generated query looks fine and should be reliable. I'm not quite sure what your schema is doing with the NOW(2) syntax for your defaults, and maybe that's causing it to not work as expected. Normally I'd expect to see something like this.

  `created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  `updated_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,

On an unrelated note, the only MySQLism that might get you is that pre-8.0 doesn't use true utf-8 by default. You should probably set your tables with DEFAULT CHARSET=utf8mb4 COLLATE= utf8mb4_general_ci to get real support.

from flipt.

sagikazarmark avatar sagikazarmark commented on May 19, 2024 1

I couldn't actually get the tests to run following the instructions above, because the schema migrations failed. After fixing them (#304), the tests are still failing though.

One possible reason for that is mysql timestamps store seconds and it looks like records are created too quickly (and the query orders records by the creation timestamp):

Screenshot 2020-06-21 at 17 23 53

(Side note: this wouldn't be a problem if you used a lexographically sortable ID, like ULID or KSUID, because MySQL uses the primary key for ordering)

Another error I see is also related to time:

    TestUpdateRuleAndDistribution: rule_test.go:402:
        	Error Trace:	rule_test.go:402
        	Error:      	Not equal:
        	            	expected: 1592752733
        	            	actual  : 1592752734
        	Test:       	TestUpdateRuleAndDistribution

Generally, I think time is not a reliable way to order records. If order doesn't actually matter, then I would probably rewrite the tests to ensure each expected item can be found in the result set, regardless of the order.

Alternatively, you could mock out time and fake sleep between inserts. That requires you to manage creation and update timestamps, but that's usually in fact a good idea, to not rely on the database, but always set the timestamp from the code which guarantees you will always get the same result.

from flipt.

derekperkins avatar derekperkins commented on May 19, 2024

This is a blocker in our evaluation of flipt

from flipt.

sagikazarmark avatar sagikazarmark commented on May 19, 2024

Just an idea: maybe consider using an ORM for a DB storage implementation? There aren't many good ORMs for Go unfortunately, but this looks promising: https://github.com/facebookincubator/ent

Normally, I'm not a huge fan of ORMs because they hurt application architecture and gives less experienced developers the wrong ideas, but as an implementation of an interface they work great, because they allow you to work rapidly.

from flipt.

markphelps avatar markphelps commented on May 19, 2024

@derekperkins @sagikazarmark I am extremely close to shipping this!.. However, I could actually use some help figuring out why a couple test cases are failing intermittently, but only when running with MySQL.

Would either of you (or anyone else) be able to help? My MySQL skills are a bit rusty.

You can see one of the failures in this Action run: https://github.com/markphelps/flipt/runs/791147606?check_suite_focus=true#step:6:632

And here's another time when running locally:

=== RUN   TestGetEvaluationDistributions
    TestGetEvaluationDistributions: evaluation_test.go:171: 
        	Error Trace:	evaluation_test.go:171
        	Error:      	Not equal: 
        	            	expected: "40c07e69-dff1-41df-84c8-3f701c965577"
        	            	actual  : "740b2abd-b474-451d-b3e5-70f953f38ae5"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-40c07e69-dff1-41df-84c8-3f701c965577
        	            	+740b2abd-b474-451d-b3e5-70f953f38ae5
        	Test:       	TestGetEvaluationDistributions
    TestGetEvaluationDistributions: evaluation_test.go:172: 
        	Error Trace:	evaluation_test.go:172
        	Error:      	Not equal: 
        	            	expected: "foo"
        	            	actual  : "bar"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-foo
        	            	+bar
        	Test:       	TestGetEvaluationDistributions
    TestGetEvaluationDistributions: evaluation_test.go:177: 
        	Error Trace:	evaluation_test.go:177
        	Error:      	Not equal: 
        	            	expected: "740b2abd-b474-451d-b3e5-70f953f38ae5"
        	            	actual  : "40c07e69-dff1-41df-84c8-3f701c965577"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-740b2abd-b474-451d-b3e5-70f953f38ae5
        	            	+40c07e69-dff1-41df-84c8-3f701c965577
        	Test:       	TestGetEvaluationDistributions
    TestGetEvaluationDistributions: evaluation_test.go:178: 
        	Error Trace:	evaluation_test.go:178
        	Error:      	Not equal: 
        	            	expected: "bar"
        	            	actual  : "foo"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-bar
        	            	+foo
        	Test:       	TestGetEvaluationDistributions
--- FAIL: TestGetEvaluationDistributions (0.03s)
=== RUN   TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:262: 
        	Error Trace:	evaluation_test.go:262
        	Error:      	Not equal: 
        	            	expected: "e84b27ab-69a2-4f65-b5f9-baa33f95d97c"
        	            	actual  : "77b5e51a-e419-4eef-afc3-b09896b1e1a3"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-e84b27ab-69a2-4f65-b5f9-baa33f95d97c
        	            	+77b5e51a-e419-4eef-afc3-b09896b1e1a3
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:263: 
        	Error Trace:	evaluation_test.go:263
        	Error:      	Not equal: 
        	            	expected: "foo"
        	            	actual  : "bar"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-foo
        	            	+bar
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:264: 
        	Error Trace:	evaluation_test.go:264
        	Error:      	Not equal: 
        	            	expected: 80
        	            	actual  : 20
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:268: 
        	Error Trace:	evaluation_test.go:268
        	Error:      	Not equal: 
        	            	expected: "77b5e51a-e419-4eef-afc3-b09896b1e1a3"
        	            	actual  : "e84b27ab-69a2-4f65-b5f9-baa33f95d97c"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-77b5e51a-e419-4eef-afc3-b09896b1e1a3
        	            	+e84b27ab-69a2-4f65-b5f9-baa33f95d97c
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:269: 
        	Error Trace:	evaluation_test.go:269
        	Error:      	Not equal: 
        	            	expected: "bar"
        	            	actual  : "foo"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-bar
        	            	+foo
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:270: 
        	Error Trace:	evaluation_test.go:270
        	Error:      	Not equal: 
        	            	expected: 20
        	            	actual  : 80
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:301: 
        	Error Trace:	evaluation_test.go:301
        	Error:      	Not equal: 
        	            	expected: "e84b27ab-69a2-4f65-b5f9-baa33f95d97c"
        	            	actual  : "77b5e51a-e419-4eef-afc3-b09896b1e1a3"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-e84b27ab-69a2-4f65-b5f9-baa33f95d97c
        	            	+77b5e51a-e419-4eef-afc3-b09896b1e1a3
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:302: 
        	Error Trace:	evaluation_test.go:302
        	Error:      	Not equal: 
        	            	expected: "foo"
        	            	actual  : "bar"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-foo
        	            	+bar
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:303: 
        	Error Trace:	evaluation_test.go:303
        	Error:      	Not equal: 
        	            	expected: 80
        	            	actual  : 20
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:307: 
        	Error Trace:	evaluation_test.go:307
        	Error:      	Not equal: 
        	            	expected: "77b5e51a-e419-4eef-afc3-b09896b1e1a3"
        	            	actual  : "e84b27ab-69a2-4f65-b5f9-baa33f95d97c"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-77b5e51a-e419-4eef-afc3-b09896b1e1a3
        	            	+e84b27ab-69a2-4f65-b5f9-baa33f95d97c
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:308: 
        	Error Trace:	evaluation_test.go:308
        	Error:      	Not equal: 
        	            	expected: "bar"
        	            	actual  : "foo"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-bar
        	            	+foo
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
    TestGetEvaluationDistributions_MaintainOrder: evaluation_test.go:309: 
        	Error Trace:	evaluation_test.go:309
        	Error:      	Not equal: 
        	            	expected: 20
        	            	actual  : 80
        	Test:       	TestGetEvaluationDistributions_MaintainOrder
--- FAIL: TestGetEvaluationDistributions_MaintainOrder (0.04s)

To Reproduce

  1. Checkout the mysql2 branch
  2. Run mysql:latest in a container like:
➜ docker run -p 3306:3306 -e MYSQL_DATABASE=flipt_test -e MYSQL_USER=mysql -e MYSQL_PASSWORD=password -e MYSQL_ALLOW_EMPTY_PASSWORD=true mysql:latest
  1. Run the tests a few times with DB_URL=mysql://mysql:password@localhost:3306/flipt_test make test
  2. Note intermittent failures

Ideas

I'm assuming the issue has to do with inconsistent result ordering being return from the following line:
https://github.com/markphelps/flipt/blob/e8a5e2055340e6bcb194ed58b6fbd59ecc0af627/storage/db/common/evaluation.go#L101-L106

Which results in this SQL query:

SELECT d.id, d.rule_id, d.variant_id, d.rollout, v.`key` FROM distributions d JOIN variants v ON (d.variant_id = v.id) WHERE d.rule_id = ? ORDER BY d.created_at ASC

This seems like a pretty basic query.. so I'm not sure what I'm doing wrong? Again, this works fine in Postgres and SQLite, so im not sure if there is some MySQLism I am missing?

Here is the schema that I'm using for MySQL btw: https://github.com/markphelps/flipt/blob/mysql2/config/migrations/mysql/0_initial.up.sql

Any pointers would be greatly appreciated 🙇🏻

from flipt.

sagikazarmark avatar sagikazarmark commented on May 19, 2024

Further looking into the topic, it looks like mysql 8 actually supports storing fractional seconds with the original syntax I removed in #304, but my guess for the breaking tests is still wrong order because of time.

I guess you could add support for mysql 5.x and 8 separately, but I'm not sure it's worth it.

from flipt.

markphelps avatar markphelps commented on May 19, 2024

@sagikazarmark @derekperkins Thanks for both of your help! I pushed a 'beta' version that supports MySQL to DockerHub:

docker pull markphelps/flipt:mysql

Please try it out if you can. I want to verify it works as expected before merging it into master and cutting a release version.

You can set it to connect to an existing MySQL DB by running it like so:

docker run -d \
    -p 8080:8080 \
    -p 9000:9000 \
    -e FLIPT_DB_URL=mysql://{user}:{password}@localhost:3306/{db} \
    markphelps/flipt:mysql

Here's more info about configuration and running with Docker.

from flipt.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.