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);
added 1 commit
- 9b179a3f - (proto): Posts, Comments and Subscriptions/Subscribers delegates
added 1 commit
- 31e5b67d - (proto): Move to Artifacts, comments should snapshot entity
added 1 commit
- 0a56c136 - (proto): Move to Artifacts, comments should snapshot entity
added 1 commit
- a58db9ea - (chore): Key serialization, snapshotable comments
added 1 commit
- f19ff274 - (chore): Always use delegates, fix comments and subs, CLI utility
added 1 commit
- 11cf756b - (refactor): Adapt to ban implementation; queue runner
added 1 commit
- 20876b2f - (test): Add Channels\Manager new methods and Channels\Snapshot
added 1 commit
- 64ee6ef2 - (feat): ElasticSearch Snapshot/Delete/Restore
unmarked as a Work In Progress
changed the description
added 1 commit
- e504da6e - (feat): Restore inverse relationship on subs
- Resolved by Mark Harding
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) { - Owner
Have we not now lost the paging functionality from https://gitlab.com/minds/engine/blob/master/Core/Channels/Delegates/DeleteArtifacts.php#L111?
- Resolved by Emiliano Balbuena
- Resolved by Emiliano Balbuena
- Last updated by Emiliano Balbuena
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) - Owner
- Owner
Also should pass $opts as array vs $user_guid. We really need strict interfaces for Repository classes
- Owner
I see we also have getList below which seems very similar, perhaps combine functionality.
changed this line in version 19 of the diff
- Last updated by Emiliano Balbuena
304 312 ]; 305 313 } 306 314 315 /** 316 * @return string[] 317 */ 318 public function getSnapshotable() - Owner
Not sure about this... Can we not just serialize the object? This feels really verbose and hard to maintain in the future.
changed this line in version 23 of the diff
- Last updated by Emiliano Balbuena
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) - Owner
Why this change?
changed this line in version 23 of the diff
added 2 commits
added 257 commits
- 4a04ab7c...8bd9d70c - 256 commits from branch
minds:master
- dd0940ad - Merge branch 'master' into sprint/Wire.feat.ban-snapshot-1
- 4a04ab7c...8bd9d70c - 256 commits from branch
added 1 commit
added 1 commit
- 1a28364e - (feat): Process subscriptions/subscribers by batches
- Resolved by Emiliano Balbuena
- Resolved by Emiliano Balbuena
- Last updated by Emiliano Balbuena
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) - 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).
changed this line in version 32 of the diff
- Resolved by Emiliano Balbuena
- Last updated by Emiliano Balbuena
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); - 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 changed this line in version 32 of the diff
- Last updated by Emiliano Balbuena
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) - Owner
I think this can be simplified with an
update_by_query
command and add/remove a deleted boolean. changed this line in version 32 of the diff
- Resolved by Emiliano Balbuena
- Resolved by Emiliano Balbuena
- Last updated by Emiliano Balbuena
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) - Owner
I don't think we need to snapshot these either
changed this line in version 33 of the diff
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) - Owner
We want to keep the global indexes of deleted users, so lets not delete on ban (snapshot/restore can be removed)
- 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
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
- 8c808555...9b4c685a - 15 commits from branch
added 9 commits
- 6cba56f6...678abba9 - 8 commits from branch
minds:master
- a552f1de - Merge branch 'master' into sprint/Wire.feat.ban-snapshot-1
- 6cba56f6...678abba9 - 8 commits from branch
added 1 commit
- 03070565 - (refactor): Add hide() and optimize/simplify processes
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
- 85776525...a11370c5 - 24 commits from branch
added 1 commit
- 1b9c9dd7 - (chore): Use string as queue name; use DI for Channels classes