Skip to content

Next

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
    • Help
    • Submit feedback
    • Contribute to GitLab
    • Switch to GitLab Next
  • Sign in / Register
Minds Backend - Engine
Minds Backend - Engine
  • Project
    • Project
    • Details
    • Activity
    • Releases
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
    • Locked Files
  • Issues 146
    • Issues 146
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Merge Requests 35
    • Merge Requests 35
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Registry
    • Registry
  • Packages
    • Packages
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • Minds
  • Minds Backend - EngineMinds Backend - Engine
  • Merge Requests
  • !120

Open
Opened 2 months ago by Emiliano Balbuena@edgebal
  • Report abuse
Report abuse

Ban and artifacts snapshots

CQL Schema changes:

CREATE TABLE minds.user_snapshots (
  user_guid varint,
  type text,
  key text,
  json_data text,
  PRIMARY KEY (user_guid, type, key)
) WITH CLUSTERING ORDER BY (type ASC, key ASC);
Edited 2 months ago by Emiliano Balbuena

Check out, review, and merge locally

Step 1. Fetch and check out the branch for this merge request

git fetch https://gitlab.com/edgebal/engine.git sprint/Wire.feat.ban-snapshot-1
git checkout -b edgebal/engine-sprint/Wire.feat.ban-snapshot-1 FETCH_HEAD

Step 2. Review the changes locally

Step 3. Merge the branch and fix any conflicts that come up

git fetch origin
git checkout origin/master
git merge --no-ff edgebal/engine-sprint/Wire.feat.ban-snapshot-1

Step 4. Push the result of the merge to GitLab

git push origin master

Note that pushing to GitLab requires write access to this repository.

Tip: You can also checkout merge requests locally by following these guidelines.

Request to merge edgebal:sprint/Wire.feat.ban-snapshot-1 into master
The source branch is 3 commits behind the target branch
Open in Web IDE
  • Email patches
  • Plain diff
Pipeline #65918050 (#554) running for 1b9c9dd7 on edgebal:sprint/Wire.feat.ban-snapshot-1
      Requires 2 more approvals.
      Ready to be merged automatically. Ask someone with write access to this repository to merge this request

      Allows commits from members who can merge to the target branch

      Deletes source branch

      • Discussion 22
      • Commits 40
      • Pipelines 37
      • Changes 62
      8/17 discussions resolved
      • Loading...
      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 9b179a3f - (proto): Posts, Comments and Subscriptions/Subscribers delegates

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 31e5b67d - (proto): Move to Artifacts, comments should snapshot entity

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 0a56c136 - (proto): Move to Artifacts, comments should snapshot entity

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • a58db9ea - (chore): Key serialization, snapshotable comments

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • f19ff274 - (chore): Always use delegates, fix comments and subs, CLI utility

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • b8357327 - (test): Fix existing tests

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 11cf756b - (refactor): Adapt to ban implementation; queue runner

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 23ffe58c - (test): Fix existing spec tests

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 20876b2f - (test): Add Channels\Manager new methods and Channels\Snapshot

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 64ee6ef2 - (feat): ElasticSearch Snapshot/Delete/Restore

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 3a96087d - (test): Fix spec tests

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 899b3d9a - (test): Artifacts Delegate spec tests

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 3655dc95 - (test): Ban/Unban spec test

        Compare with previous version

      • Emiliano Balbuena @edgebal unmarked as a Work In Progress 2 months ago

        unmarked as a Work In Progress

      • Emiliano Balbuena @edgebal changed the description 2 months ago

        changed the description

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • deea7f99 - (refactor): Use DeleteByQuery

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 50ad26f7 - (test): Elasticsearch delegate spec tests

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 6742969e - (test): Posts delegate spec test

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • e504da6e - (feat): Restore inverse relationship on subs

        Compare with previous version

      • Mark Harding
        Mark Harding @markeharding started a discussion on the diff 2 months ago
        Resolved by Mark Harding 2 months ago
      • Mark Harding
        Mark Harding @markeharding started a discussion on the diff 2 months ago
        Core/Channels/Delegates/Artifacts/SubscribersDelegate.php
        125 * @return bool
        126 */
        127 public function delete($userGuid)
        128 {
        129 try {
        130 $cql = "SELECT * FROM friendsof WHERE key = ?";
        131 $values = [
        132 (string) $userGuid,
        133 ];
        134
        135 $prepared = new Custom();
        136 $prepared->query($cql, $values);
        137
        138 $rows = $this->db->request($prepared);
        139
        140 foreach ($rows as $row) {
        • Mark Harding
          Mark Harding @markeharding · 2 months ago
          Owner

          Have we not now lost the paging functionality from https://gitlab.com/minds/engine/blob/master/Core/Channels/Delegates/DeleteArtifacts.php#L111?

        Please register or sign in to reply
      • Mark Harding
        Mark Harding @markeharding started a discussion on the diff 2 months ago
        Resolved by Emiliano Balbuena 2 months ago
      • Mark Harding
        Mark Harding @markeharding started a discussion on the diff 2 months ago
        Resolved by Emiliano Balbuena 2 months ago
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 months ago
        Last updated by Emiliano Balbuena 2 months ago
        Core/Channels/Snapshots/Repository.php
        22 * @param CassandraClient $db
        23 */
        24 public function __construct(
        25 $db = null
        26 )
        27 {
        28 $this->db = $db ?: Di::_()->get('Database\Cassandra\Cql');
        29 }
        30
        31 /**
        32 * @param string|int $userGuid
        33 * @return \Generator
        34 * @yields Snapshot[]
        35 * @throws \Exception
        36 */
        37 public function getAll($userGuid)
        • Mark Harding
          Mark Harding @markeharding · 2 months ago
          Owner

          getList for Repository as with https://gitlab.com/minds/engine/blob/master/Core/Feeds/Top/Repository.php#L35 , https://gitlab.com/minds/engine/blob/master/Core/Suggestions/Repository.php#L26 etc

        • Mark Harding
          Mark Harding @markeharding · 2 months ago
          Owner

          Also should pass $opts as array vs $user_guid. We really need strict interfaces for Repository classes

        • Mark Harding
          Mark Harding @markeharding · 2 months ago
          Owner

          I see we also have getList below which seems very similar, perhaps combine functionality.

        • Emiliano Balbuena @edgebal changed this line in version 19 of the diff 2 months ago

          changed this line in version 19 of the diff

        Please register or sign in to reply
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 months ago
        Last updated by Emiliano Balbuena 2 months ago
        Core/Comments/Comment.php
        304 312 ];
        305 313 }
        306 314
        315 /**
        316 * @return string[]
        317 */
        318 public function getSnapshotable()
        • Mark Harding
          Mark Harding @markeharding · 2 months ago
          Owner

          Not sure about this... Can we not just serialize the object? This feels really verbose and hard to maintain in the future.

        • Emiliano Balbuena @edgebal changed this line in version 23 of the diff 2 months ago

          changed this line in version 23 of the diff

        Please register or sign in to reply
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 months ago
        Last updated by Emiliano Balbuena 2 months ago
        Core/Comments/Manager.php
        94 94 /**
        95 95 * Adds a comment and triggers creation events
        96 96 * @param Comment $comment
        97 * @param bool $triggerEvents
        97 98 * @return bool
        98 99 * @throws BlockedUserException
        99 * @throws \Exception
        100 * @throws \Minds\Exceptions\StopEventException
        100 101 */
        101 public function add(Comment $comment)
        102 public function add(Comment $comment, $triggerEvents = true)
        • Mark Harding
          Mark Harding @markeharding · 2 months ago
          Owner

          Why this change?

        • Emiliano Balbuena @edgebal changed this line in version 23 of the diff 2 months ago

          changed this line in version 23 of the diff

        Please register or sign in to reply
      • Emiliano Balbuena @edgebal added 2 commits 2 months ago

        added 2 commits

        • 12a1a2ab - (chore): Refactor big if statements
        • 0a81bdb0 - (refactor): Use getList and $opts

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 63bf7ddc - (refactor): Separate ban logic from Manager

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • ff7e6ca0 - (feat): Deletion as background task

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 months ago

        added 1 commit

        • 78656a10 - (feat): Deletion as background task

        Compare with previous version

      • Emiliano Balbuena @edgebal added 2 commits 2 months ago

        added 2 commits

        • 852e7375 - (fix): User entity deletion should delete artifacts
        • 4a04ab7c - (refactor): Serialize comment during snapshot

        Compare with previous version

      • Emiliano Balbuena @edgebal added 257 commits 2 weeks ago

        added 257 commits

        • 4a04ab7c...8bd9d70c - 256 commits from branch minds:master
        • dd0940ad - Merge branch 'master' into sprint/Wire.feat.ban-snapshot-1

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 weeks ago

        added 1 commit

        • 3af444b0 - (refactor): Use subscribe()/unSubscribe() methods on snapshots (#461)

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 weeks ago

        added 1 commit

        • 4e56b2c7 - (fix): Force comment deleting on queue

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 weeks ago

        added 1 commit

        • 1a28364e - (feat): Process subscriptions/subscribers by batches

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 2 weeks ago

        added 1 commit

        • ec9ed48a - (test): Fix spec tests

        Compare with previous version

      • Emiliano Balbuena @edgebal added 2 commits 2 weeks ago

        added 2 commits

        • 9023c045 - (feat): Cassandra Scroll helper class
        • 8c808555 - (refactor): Use Scroll class to traverse friends tables

        Compare with previous version

      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 weeks ago
        Resolved by Emiliano Balbuena 3 minutes ago
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 weeks ago
        Resolved by Emiliano Balbuena 26 minutes ago
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 weeks ago
        Last updated by Emiliano Balbuena 1 week ago
        Core/Channels/Ban.php
        125 foreach (static::BAN_DELEGATES as $delegateClassName) {
        126 try {
        127 $delegate = $this->artifactsDelegatesFactory->build($delegateClassName);
        128 $done = $delegate->snapshot($userGuid);
        129
        130 if (!$done) {
        131 throw new \Exception("{$delegateClassName} ban cleanup snapshot failed for {$userGuid}");
        132 }
        133 } catch (\Exception $e) {
        134 // TODO: Fail?
        135 error_log((string) $e);
        136 $snapshotCreated = false;
        137 }
        138 }
        139
        140 // Delete (only if all snapshots were successful)
        • Mark Harding
          Mark Harding @markeharding · 2 weeks ago
          Owner

          q. Do they need to be run in two batches (ie. all snapshots and then all deletes), or can we just do in pair (one foreach loop with a snapshot/delete in).

        • Emiliano Balbuena @edgebal changed this line in version 32 of the diff 1 week ago

          changed this line in version 32 of the diff

        Please register or sign in to reply
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 weeks ago
        Resolved by Emiliano Balbuena 3 minutes ago
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 weeks ago
        Last updated by Emiliano Balbuena 1 week ago
        Core/Channels/Delegates/Artifacts/ElasticsearchDocumentsDelegate.php
        47
        48 return true;
        49 }
        50
        51 /**
        52 * @param $userGuid
        53 */
        54 protected function snapshotUser($userGuid)
        55 {
        56 $params = [
        57 'index' => $this->config->get('elasticsearch')['index'],
        58 'type' => 'user',
        59 'id' => $userGuid
        60 ];
        61
        62 $document = $this->elasticsearch->getClient()->get($params);
        • Mark Harding
          Mark Harding @markeharding · 2 weeks ago
          Owner

          I don't think we need to do a snapshot here. As there is nothing to restore from. All we need to do is an update_by_query request to remove the deleted boolean

        • Emiliano Balbuena @edgebal changed this line in version 32 of the diff 1 week ago

          changed this line in version 32 of the diff

        Please register or sign in to reply
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 weeks ago
        Last updated by Emiliano Balbuena 1 week ago
        Core/Channels/Delegates/Artifacts/ElasticsearchDocumentsDelegate.php
        171 if (count($bulk) > 2000) {
        172 $this->elasticsearch->bulk(['body' => $bulk]);
        173 $bulk = [];
        174 }
        175 }
        176
        177 if ($bulk) {
        178 $this->elasticsearch->bulk(['body' => $bulk]);
        179 }
        180 }
        181
        182 /**
        183 * @param $userGuid
        184 * @throws \Exception
        185 */
        186 protected function restoreOwnedByUser($userGuid)
        • Mark Harding
          Mark Harding @markeharding · 2 weeks ago
          Owner

          I think this can be simplified with an update_by_query command and add/remove a deleted boolean.

        • Emiliano Balbuena @edgebal changed this line in version 32 of the diff 1 week ago

          changed this line in version 32 of the diff

        Please register or sign in to reply
      • Mark Harding
        Mark Harding @markeharding started a discussion on the diff 2 weeks ago
        Resolved by Emiliano Balbuena 1 minute ago
      • Mark Harding
        Mark Harding @markeharding started a discussion on the diff 2 weeks ago
        Resolved by Emiliano Balbuena 1 minute ago
      • Mark Harding
        Mark Harding @markeharding started a discussion on an old version of the diff 2 weeks ago
        Last updated by Emiliano Balbuena 1 week ago
        Core/Channels/Delegates/Artifacts/PostsDelegate.php
        35 * @param CassandraClient $db
        36 */
        37 public function __construct(
        38 $repository = null,
        39 $db = null
        40 )
        41 {
        42 $this->repository = $repository ?: new Repository();
        43 $this->db = $db ?: Di::_()->get('Database\Cassandra\Cql');
        44 }
        45
        46 /**
        47 * @param string|int $userGuid
        48 * @return bool
        49 */
        50 public function snapshot($userGuid)
        • Mark Harding
          Mark Harding @markeharding · 2 weeks ago
          Owner

          I don't think we need to snapshot these either

        • Emiliano Balbuena @edgebal changed this line in version 33 of the diff 1 week ago

          changed this line in version 33 of the diff

        Please register or sign in to reply
      • Mark Harding
        Mark Harding @markeharding started a discussion on the diff 2 weeks ago
        Core/Channels/Delegates/Artifacts/UserIndexesDelegate.php
        27 * @param CassandraClient $db
        28 */
        29 public function __construct(
        30 $repository = null,
        31 $db = null
        32 )
        33 {
        34 $this->repository = $repository ?: new Repository();
        35 $this->db = $db ?: Di::_()->get('Database\Cassandra\Cql');
        36 }
        37
        38 /**
        39 * @param string|int $userGuid
        40 * @return bool
        41 */
        42 public function snapshot($userGuid)
        • Mark Harding
          Mark Harding @markeharding · 2 weeks ago
          Owner

          We want to keep the global indexes of deleted users, so lets not delete on ban (snapshot/restore can be removed)

        Please register or sign in to reply
      • Mark Harding
        Mark Harding @markeharding · 2 weeks ago
        Owner

        I think we can greatly simplify/optimise the delete/snapshot stuff

        • No need to snapshot/restore personal indexes
        • No need to snapshot/restore lookup indexes
        • No need to snapshot/restore entities (they can just add the 'delete' boolean to them)
        • No need to download from ES (a boolean on all the owner_guids documents should suffice)
        • There needs to be a distinction between hiding/deleting. I would just add another function called hide(), and if it is a delete operation for that delegate, it will just pass though as delete
      • Emiliano Balbuena @edgebal added 16 commits 1 week ago

        added 16 commits

        • 8c808555...9b4c685a - 15 commits from branch minds:master
        • 6cba56f6 - Merge remote-tracking branch 'upstream/master' into sprint/Wire.feat.ban-snapshot-1

        Compare with previous version

      • Emiliano Balbuena @edgebal added 9 commits 1 week ago

        added 9 commits

        • 6cba56f6...678abba9 - 8 commits from branch minds:master
        • a552f1de - Merge branch 'master' into sprint/Wire.feat.ban-snapshot-1

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 1 week ago

        added 1 commit

        • 03070565 - (refactor): Add hide() and optimize/simplify processes

        Compare with previous version

      • Emiliano Balbuena @edgebal added 2 commits 1 week ago

        added 2 commits

        • fea5e2ab - (chore): Code cleanup; (fix): Process hidden user entities
        • a253b54e - (chore): Posts delegate should be UserEntities

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 1 week ago

        added 1 commit

        • 0d7a2e1e - (test): Fix spec tests

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 1 week ago

        added 1 commit

        • 85776525 - (refactor): Snapshot CLI allow banning

        Compare with previous version

      • Emiliano Balbuena @edgebal added 25 commits 2 hours ago

        added 25 commits

        • 85776525...a11370c5 - 24 commits from branch minds:master
        • a82b01ae - Merge remote-tracking branch 'upstream/master' into sprint/Wire.feat.ban-snapshot-1

        Compare with previous version

      • Emiliano Balbuena @edgebal added 1 commit 4 minutes ago

        added 1 commit

        • 1b9c9dd7 - (chore): Use string as queue name; use DI for Channels classes

        Compare with previous version

      • You're only seeing other activity in the feed. To add a comment, switch to one of the following options.
      Please register or sign in to reply
      Assignee
      Emiliano Balbuena's avatar Emiliano Balbuena @edgebal
      Assign to
      None
      Milestone
      None
      Assign milestone
      None
      Time tracking
      No estimate or time spent
      0
      Labels
      None
      Assign labels
      • View project labels
      Lock merge request
      Unlocked
      2
      2 participants
      user avatar
      Emiliano Balbuena
      user avatar
      Mark Harding
      Reference: minds/engine!120