Usage factor persistence #4802

shadeofblue · 2019-10-17T14:36:16Z

closes GR-178

mbenke · 2019-10-17T15:15:28Z

It might be worthwhile to add some tests along the lines of TestLocalRank

mbenke · 2019-10-17T14:46:38Z

golem/database/schemas/040_usage_factor.py

+        usage_factor = pw.FloatField(default=1.0)
+
+        class Meta:
+            db_table = "usagefactor"


Does it need a separate model/table? Isn't it better to just add a field in LocalRank?

I'm unsure but maybe I know too little about the marketplace in general -> how does a usage factor relate to LocalRank? is a usage factor always accompanied by a local rank record? does one implicitly assume the other or can they exist independently? are the properties in the LocalRank used for the same purpose as the usage factor?

@mbenke ^ ?

LocalRank is used basically for the same purpose as usage factors, i.e. provider choice.

golem/model.py

kmazurek · 2019-10-17T15:27:39Z

golem/database/schemas/040_usage_factor.py

+SCHEMA_VERSION = 40
+
+try:
+    import playhouse.postgres_ext as pw_pext


Is this import required? How is it used here?

it was added automatically by peewee_migrate...

mbenke · 2019-10-18T10:05:01Z

Also, TestRequestorMarketStrategy seems to need to inherit DatabaseFixture now

kubkon

Definitely not an expert on this, but here's a couple of nitpicks.

kubkon · 2019-10-21T07:15:44Z

golem/database/schemas/040_usage_factor.py

+        created_date = pw.UTCDateTimeField(default=dt.datetime.now)
+        modified_date = pw.UTCDateTimeField(default=dt.datetime.now)


I assume that we somehow use these for tracking usage factors in time? If not, are they not somewhat redundant?

these are inherited from BaseModel so, as standard feature of all golem models, so to say...

kubkon · 2019-10-21T07:20:08Z

golem/marketplace/wasm_marketplace.py

+            model.UsageFactor.provider_node_id == provider_id).first()
+
+        r = delta * usage_factor.usage_factor
+        logger.info("RWMS: adjust R for provider %s: %.3f -> %.3f",


Silly question but what's "RWMS"? Weighted RMS perhaps?

@kubkon no idea, as you may see, this logging line was just copied from the other place

I believe it's more of RequestorWasmMarketStrategy than whatever else though, @kubkon

Indeed, RWMS stands for RequestorWasmMarketStrategy. We use this abbreviation in log messages to make them shorter, but perhaps this abbreviation renders them less readable.

kmazurek · 2019-10-21T09:27:01Z

golem/database/schemas/041_usage_factor.py

+import datetime as dt
+import peewee as pw
+
+SCHEMA_VERSION = 40


Note: the schema versions were shifted in #4807. I see you merged with develop, there's now a unit test which detects schema version collisions. Also, I'm adding a migration in #4811 - the race is on.

@zakaprov yeah, I noticed ... I find our migration system horribly broken in that regard... migration numbering (as opposed to their contents) should NOT be something you have to take into account when resolving merge conflicts... :/

SCHEMA_VERSION can be dropped altogether. Filenames decide the order that migrations are performed in.

If you believe that numbering is horribly broken please propose an alternative solution.

@mfranciszkiewicz numbering itself is not the issue ... conflict resolution of those numbered migrations is the issue... it's okay for the numbering to determine the order but the existence of two migrations that share the same number should not be an issue. and as long as those migrations don't modify the same model, they should be able to co-exist...

good example could be Django's south (later just renamed to "django migrations") which handles such cases gracefully... though, of course, it being a django-specific module, it cannot/should not be used in a project like ours ...

@zakaprov lost it ;)

shadeofblue · 2019-10-21T16:49:54Z

It might be worthwhile to add some tests along the lines of TestLocalRank

@mbenke some tests added ...

- the need to explicitly .save() peewee factory instances + tests for WASM marketplace

codecov · 2019-10-22T12:40:13Z

Codecov Report

Merging #4802 into develop will decrease coverage by 0.08%.
The diff coverage is 87.09%.

@@             Coverage Diff             @@
##           develop    #4802      +/-   ##
===========================================
- Coverage    89.42%   89.33%   -0.09%     
===========================================
  Files          226      216      -10     
  Lines        20540    20126     -414     
===========================================
- Hits         18367    17980     -387     
+ Misses        2173     2146      -27

shadeofblue added 2 commits October 17, 2019 11:58

+ model.UsageFactor

13d224f

use the database to store the usage factors

b9b2f8a

shadeofblue requested review from kmazurek, kubkon and mbenke October 17, 2019 14:36

shadeofblue self-assigned this Oct 17, 2019

mbenke reviewed Oct 17, 2019

View reviewed changes

kmazurek reviewed Oct 17, 2019

View reviewed changes

kubkon reviewed Oct 21, 2019

View reviewed changes

shadeofblue added 4 commits October 21, 2019 10:08

lint...

1b5b5b7

Merge branch 'develop' into usage-factor-persistence

5ca4435

merge followup (migration rename)

1aba4d0

fix tests

37ff2b2

kmazurek reviewed Oct 21, 2019

View reviewed changes

ugh...

aa64b9d

+ base PeeweeModelFactory

d4ead7f

- the need to explicitly .save() peewee factory instances + tests for WASM marketplace

shadeofblue force-pushed the usage-factor-persistence branch from bfaf357 to d4ead7f Compare October 22, 2019 08:23

proper test setup

77e7d39

mbenke approved these changes Oct 22, 2019

View reviewed changes

shadeofblue added 2 commits October 22, 2019 15:30

Merge branch 'develop' into usage-factor-persistence

24ca42c

fix migration numbering ... :p ...

8deb57c

shadeofblue merged commit 93f22e9 into develop Oct 22, 2019

shadeofblue deleted the usage-factor-persistence branch October 22, 2019 14:39

This was referenced Oct 24, 2019

Fix division and limit benchmark-based usage factors #4827

Merged

limit benchmark-based usage factors #4803

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Usage factor persistence #4802

Usage factor persistence #4802

shadeofblue commented Oct 17, 2019

mbenke commented Oct 17, 2019

mbenke Oct 17, 2019

shadeofblue Oct 21, 2019

shadeofblue Oct 21, 2019

mbenke Oct 22, 2019

kmazurek Oct 17, 2019

shadeofblue Oct 21, 2019

mbenke commented Oct 18, 2019

kubkon left a comment

kubkon Oct 21, 2019

shadeofblue Oct 21, 2019

kubkon Oct 21, 2019

shadeofblue Oct 21, 2019

shadeofblue Oct 21, 2019

mbenke Oct 22, 2019

kmazurek Oct 21, 2019

shadeofblue Oct 21, 2019 •

edited

Loading

mfranciszkiewicz Oct 21, 2019

shadeofblue Oct 21, 2019 •

edited

Loading

shadeofblue Oct 22, 2019

shadeofblue commented Oct 21, 2019

codecov bot commented Oct 22, 2019 •

edited

Loading

		created_date = pw.UTCDateTimeField(default=dt.datetime.now)
		modified_date = pw.UTCDateTimeField(default=dt.datetime.now)

Usage factor persistence #4802

Usage factor persistence #4802

Conversation

shadeofblue commented Oct 17, 2019

mbenke commented Oct 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbenke commented Oct 18, 2019

kubkon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shadeofblue Oct 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shadeofblue Oct 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shadeofblue commented Oct 21, 2019

codecov bot commented Oct 22, 2019 • edited Loading

Codecov Report

shadeofblue Oct 21, 2019 •

edited

Loading

shadeofblue Oct 21, 2019 •

edited

Loading

codecov bot commented Oct 22, 2019 •

edited

Loading