Separate DataStore and TracingStore #3281

fm3 · 2018-09-25T13:07:18Z

URL of deployed dev instance (used for testing):

https://___.webknossos.xyz

Notes

note that tracing related api routes no longer have the data/ prefix
note that you might have to refresh your schema so that initialData is inserted again (with tracingStore info)

TODO:

Steps to test:

trace some, both in volume and skeleton
set up tracingStore as standalone app (→setup), should still be able to trace both with skeleton + volume
download + reupload volume tracing

Issues:

fixes Separate DataStore and TracingStore #3072

Updated changelog
Updated migration guide if applicable
~~[ ] Updated documentation if applicable~~
Needs datastore update after deployment
Ready for review

…to tracingstore

…cingstore

…to tracingstore

…lients

…to tracingstore

fm3 · 2018-10-05T13:09:14Z

@jstriebel @normanrz do you know what needs to be done so that we can publish the tracingstore image to dockerhub?

normanrz · 2018-10-09T08:53:16Z

Done https://hub.docker.com/r/scalableminds/webknossos-tracingstore/

daniel-wer

@philippotto I reviewed the frontend part, very good work :)
If I misunderstood something with the datastore/tracingstore url, then nevermind, but I think there are some spots in the tests where the wrong one of the two is used.
Do we know whether having to send two requests instead of one when requesting fallback segmentation data hurts performance much? Did you notice anything?

daniel-wer · 2018-10-09T09:01:28Z

MIGRATIONS.md

@@ -6,12 +6,13 @@ User-facing changes are documented in the [changelog](CHANGELOG.md).

 ## Unreleased
 - Some config keys have changed, if you overwrite them in your setup, please adapt: the `oxalis` prefix is renamed to `webKnossos` so the new keys are `webKnossos.user.time.tracingPauseInSeconds`, `webKnossos.tasks.maxOpenPerUser`, `webKnossos.newOrganizationMailingList` as well as `datastore.webKnossos.uri`, `datastore.webKnossos.secured`, `datastore.webKnossos.pingIntervalMinutes` for the datastore.
+- There is now a separate module for the tracing store, the datastore is no longer responsible for saving tracings. This module can run as a standalone application, or as a module of webknossos locally. It is recommended that you chose the option that was previously also in place for datastores. In case of a standalone datastore, the local one needs to be disabled in application.conf: `tracingstore.enabled = false` and `play.modules.disabled += "com.scalableminds.braingames.datastore.TracingStoreModule` – and in either case, the adress of the tracingstore (localhost or remote) needs to be inserted in the db in `webknossos.tracingStores`


Either always use tracing store or tracingstore

webKnossos instead of webknossos

choose instead of chose

I would also add a period to the end of the last sentence :)

daniel-wer · 2018-10-09T09:02:01Z

CHANGELOG.md

@@ -18,6 +18,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).
 - The UI for editing experience domains of users was improved. [#3254](https://github.com/scalableminds/webknossos/pull/3254)
 - The tracing layout was changed to be more compact. [#3256](https://github.com/scalableminds/webknossos/pull/3256)
 - It is no longer possible to draw outside of a viewport with the brush tool in volume tracing. [#3283](https://github.com/scalableminds/webknossos/pull/3283)
+- There is now a separate tracingstore module, the datastore is no longer responsible for saving tracings


Please add a period and link this PR

daniel-wer · 2018-10-09T09:12:48Z

app/assets/javascripts/oxalis/api/api_latest.js

@@ -729,6 +729,7 @@ class DataApi {
    const dataset = Store.getState().dataset;

    return doWithToken(token => {
+      // todo: use tracingStore maybe?


Is this todo still valid? The method description makes it look like the method can be used to download segmentation data as well? Would the request for the segmentation data need to go to the tracing- or the datastore? Or both? ^^ :/

Thanks for the comment. @fm3 and I decided to clarify the api description so that the download only refers to the dataset (and not tracing). I'm not sure whether this api is used anyway. If users voice interest in downloading volume tracings, we can add another option. Merging is hopefully not necessary.

daniel-wer · 2018-10-09T09:13:00Z

app/assets/javascripts/oxalis/api/api_v2.js

@@ -617,6 +617,7 @@ class DataApi {
    const dataset = Store.getState().dataset;

    return doWithToken(token => {
+      // todo: use tracingStore maybe?


See comment in api_latest.

daniel-wer · 2018-10-09T09:33:11Z

app/assets/javascripts/test/backend-snapshot-tests/annotations.e2e.js

@@ -150,7 +150,7 @@ async function sendUpdateActions(explorational, queue) {
  const skeletonTracingId = explorational.tracing.skeleton;
  if (skeletonTracingId == null) throw new Error("No skeleton tracing present.");
  return sendRequestWithToken(
-    `${explorational.dataStore.url}/data/tracings/skeleton/${skeletonTracingId}/update?token=`,
+    `${explorational.dataStore.url}/tracings/skeleton/${skeletonTracingId}/update?token=`,


Shouldn't this be the tracingstore url? But the e2e tests go through successfully - I'm confused ^^

It's probably because both are the same, right? But the tracingstore url would be semantically correct.

daniel-wer · 2018-10-09T09:34:56Z

app/assets/javascripts/test/model/binary/layers/wkstore_adapter.spec.js

+      .returns(Promise.resolve({ token: "token2" }));
+
+    return requestWithFallback(layer, batch).then(([buffer1, buffer2]) => {
+      // console.log("buffer1", buffer1);


These can probably be removed

daniel-wer · 2018-10-09T09:38:30Z

app/assets/javascripts/test/model/model.spec.js

@@ -66,7 +66,7 @@ test.beforeEach(t => {
  const tracingId = ANNOTATION.tracing[contentType];

  Request.receiveJSON
-    .withArgs(`${ANNOTATION.dataStore.url}/data/tracings/${contentType}/${tracingId}`)
+    .withArgs(`${ANNOTATION.dataStore.url}/tracings/${contentType}/${tracingId}`)


Shouldn't this also be the tracingstore url?

daniel-wer · 2018-10-09T09:39:39Z

app/assets/javascripts/test/sagas/save_saga.spec.js

@@ -122,7 +122,7 @@ test("SaveSaga should send request to server", t => {
  expectValueDeepEqual(
    t,
    saga.next(DATASTORE_URL),
-    call(sendRequestWithToken, `${DATASTORE_URL}/data/tracings/skeleton/1234567890/update?token=`, {
+    call(sendRequestWithToken, `${DATASTORE_URL}/tracings/skeleton/1234567890/update?token=`, {


Rename DATASTORE_URL to TRACINGSTORE_URL?

fm3 · 2018-10-09T10:00:03Z

I updated the changelog + migration guide.
The tests probably run green despite the wrong url, because in the standard case, all three urls (wK, datastore, tracingstore) are localhost:9000.
I could not see any performance problems with the double requests locally, but I guess that does not have to mean too much. But my guess is that it’ll be ok

…to tracingstore

[WIP] tracingstore module

b9bfe49

fm3 added backend infrastructure refactoring datastore-changes labels Sep 25, 2018

fm3 self-assigned this Sep 25, 2018

fm3 added 3 commits September 25, 2018 15:14

update schema to register datastores

2dd1de2

move towards tracing store module

08eb5e2

tracingstore gets its own config

c5fd581

fm3 added the frontend label Sep 25, 2018

fm3 and others added 4 commits September 25, 2018 16:14

Merge branch 'master' into tracingstore

4d1dee2

add a tracingstore to the test db

14179fa

Merge branch 'tracingstore' of github.com:scalableminds/webknossos in…

2aac287

…to tracingstore

provide volumetracing data route

3dd27c5

fm3 assigned philippotto Sep 25, 2018

fm3 changed the title ~~[WIP] Tracingstore Module~~ [WIP] Separate DataStore and TracingStore Sep 26, 2018

philippotto and others added 14 commits September 26, 2018 13:39

Merge branch 'master' of github.com:scalableminds/webknossos into tra…

0cc444e

…cingstore

fix standalone tracingstore

5b48f4e

standalone tracingstore not enabled by default

7609e3e

fix local tracingstore, sacrificing standalone-one for now

1c1488a

Merge branch 'tracingstore' of github.com:scalableminds/webknossos in…

68b6f39

…to tracingstore

use tracing store in front-end (not yet for volume tracings)

261958e

refactor buffer slicing out of pullqueue and into wkstore adapter

3e5e19c

[WIP] set volume properties on upload

729552d

Merge branch 'master' into tracingstore

a99677a

Merge branch 'tracingstore' of github.com:scalableminds/webknossos in…

134486d

…to tracingstore

in volume tracing upload, set fallback-layer dependent values

18ecaad

rename some files, separate datastore/tracingstore wk communication c…

0308c52

…lients

fix token auth for tracing store

c89d4c7

implement segmentation fallback logic in front-end

8adb435

philippotto and others added 6 commits October 5, 2018 13:46

fix wk store adapter spec

8cc8530

merge master into tracingstore

0b1a60b

fix pullqueue spec

92801f3

post-merge fix for application.conf

962a73b

Merge branch 'tracingstore' of github.com:scalableminds/webknossos in…

88a6ed4

…to tracingstore

update evolution number

7ed8674

fm3 changed the title ~~[WIP] Separate DataStore and TracingStore~~ Separate DataStore and TracingStore Oct 5, 2018

fm3 mentioned this pull request Oct 8, 2018

Versioned volume tracings #3146

Closed

fm3 added 2 commits October 9, 2018 09:59

[WIP] merge master into tracingstore

8436c82

make evolution numbers unique, fix variable unpacking in version_list.js

a1b9816

fm3 requested a review from daniel-wer October 9, 2018 08:15

fix schema version numbers

59fb8e5

daniel-wer reviewed Oct 9, 2018

View reviewed changes

update changelog + migrations.md

fd5442c

fm3 and others added 10 commits October 9, 2018 12:01

merge master into tracingstore, adapt evolution numbers

c02ce29

incorporate PR feedback

65a80c5

Merge branch 'tracingstore' of github.com:scalableminds/webknossos in…

1f98369

…to tracingstore

merge master into tracingstore

99b408d

Merge branch 'master' into tracingstore

c1eecf3

merge master into tracingstore

f28d498

merge master into tracingstore

ddb3a7d

fix update action log route for volumes

a33c4fd

lint

fd203db

Merge branch 'master' into tracingstore

7836e45

fm3 merged commit d559ee3 into master Oct 17, 2018

fm3 deleted the tracingstore branch October 17, 2018 14:16

fm3 mentioned this pull request Oct 21, 2018

Ensure shared Data Cube Cache by holding Binary Data Service in injected Singleton #3381

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate DataStore and TracingStore #3281

Separate DataStore and TracingStore #3281

fm3 commented Sep 25, 2018 •

edited

fm3 commented Oct 5, 2018 •

edited

normanrz commented Oct 9, 2018

daniel-wer left a comment

daniel-wer Oct 9, 2018

daniel-wer Oct 9, 2018

daniel-wer Oct 9, 2018

philippotto Oct 10, 2018

daniel-wer Oct 9, 2018

daniel-wer Oct 9, 2018

daniel-wer Oct 9, 2018

daniel-wer Oct 9, 2018

daniel-wer Oct 9, 2018

daniel-wer Oct 9, 2018

fm3 commented Oct 9, 2018

Separate DataStore and TracingStore #3281

Separate DataStore and TracingStore #3281

Conversation

fm3 commented Sep 25, 2018 • edited

URL of deployed dev instance (used for testing):

Notes

TODO:

Steps to test:

Issues:

fm3 commented Oct 5, 2018 • edited

normanrz commented Oct 9, 2018

daniel-wer 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fm3 commented Oct 9, 2018

fm3 commented Sep 25, 2018 •

edited

fm3 commented Oct 5, 2018 •

edited