Skip to content

Mangle key to orphan data - #35

Closed
efischer19 wants to merge 4 commits into
masterfrom
efischer/mangle_key
Closed

Mangle key to orphan data#35
efischer19 wants to merge 4 commits into
masterfrom
efischer/mangle_key

Conversation

@efischer19

Copy link
Copy Markdown
Contributor

To fix the buggy behavior reported in TNL-3880, we need to ensure
that the submission we're dropping is unlinked from the student_item
used to find it. This prevents code (say, an ORA staff tool) from
constructing a student item with an id, a course, and a problem, and
using that to find a "cleared" submission.
@efischer19 efischer19 mentioned this pull request Feb 8, 2016
@efischer19

Copy link
Copy Markdown
Contributor Author

Initial testing looks good on my devstack

@efischer19

Copy link
Copy Markdown
Contributor Author

Updated to append +orphaned to the item_id to keep values more readable.

@robrap is out today, but FYI Robert if you could take a look and weigh in here we'd appreciate it.

The problem that needs to be solved is:

  • The StudentItem and Submission models need to be unlinked, so that ORA staff tools can't find the submission after state is cleared.
  • The Submission needs to keep some data about the associated StudentItem to remain useful to analytics later.

The 3 options I've investigated are:

  • Add a deleted column to the Submission model. This is seen in Soft-delete Submissions to reset student state #33.
    • Pros: keeps all data intact for analytics, easy to handle in data downloads or other backend database queries.
    • Cons: Large data migration required, potential performance hit by adding .filter(deleted=False) to all queries on Submission.objects.
  • Set the StudentItem to None on Submission model. See Clear state option #34.
    • Pros: smaller (but still non-trivial) schema migration to remove NOT NULL constraint, no performance concerns after migration.
    • Cons: Loses data for analytics, would require adding events coverage or some other medium to track this. Maybe an entire new table? Idk.
  • Mangle the StudentItem FK as seen in this PR.
    • Pros: Minimal data loss for analytics, no migration needed, no performance concerns.
    • Cons: Not quite 100% data integrity (if len(item_id) is > 245 characters), possibly unintuitive-and-needs-documentation data format.

I personally think the mangling option addresses our problems most cleanly, but I'm curious what others think.
@cahrens @dianakhuang @maxrothman @mulby

@cahrens

cahrens commented Feb 8, 2016

Copy link
Copy Markdown

Do we know if this will still work for analytics though? When the submissions are fetched for download, I am concerned that these will not be included because the item_id does not match. Can you confirm if it will work?

@efischer19

Copy link
Copy Markdown
Contributor Author

I'll leave the actual yes/no call to @mulby, but my thinking was that it should be possible to, when exporting data from the SQL tables, do a bit of processing to match any "mangled" item_id together. Is that something we could do? The rows would look like this before processing:

mysql> select * from submissions_studentitem;
+----+----------------------------------+-----------------------------+------------------------------------------------------------------------------------------------+----------------+
| id | student_id                       | course_id                   | item_id                                                                                        | item_type      |
+----+----------------------------------+-----------------------------+------------------------------------------------------------------------------------------------+----------------+
|  1 | bdc75dee173562c356edfc0542d69c95 | course-v1:edx+ORA203+course | block-v1:edx+ORA203+course+type@openassessment+block@6e78fa380b33424586d19ae858ae1be0          | openassessment |
|  2 | bdc75dee173562c356edfc0542d69c95 | course-v1:edx+ORA203+course | block-v1:edx+ORA203+course+type@openassessment+block@6e78fa380b33424586d19ae858ae1be0+orphaned | openassessment |
+----+----------------------------------+-----------------------------+------------------------------------------------------------------------------------------------+----------------+
2 rows in set (0.00 sec)

mysql> select * from submissions_submission;
+----+--------------------------------------+----------------+----------------------------+----------------------------+-------------------------------+-----------------+
| id | uuid                                 | attempt_number | submitted_at               | created_at                 | raw_answer                    | student_item_id |
+----+--------------------------------------+----------------+----------------------------+----------------------------+-------------------------------+-----------------+
|  1 | 8ee5ad38-ce81-11e5-adef-080027880ca6 |              1 | 2016-02-08 16:32:34.818498 | 2016-02-08 16:32:34.818511 | {"parts":[{"text":"asdasd"}]} |               2 |
|  2 | a6138f0c-ce81-11e5-b8a9-080027880ca6 |              1 | 2016-02-08 16:33:13.706525 | 2016-02-08 16:33:13.706538 | {"parts":[{"text":"asdasd"}]} |               1 |
+----+--------------------------------------+----------------+----------------------------+----------------------------+-------------------------------+-----------------+
2 rows in set (0.00 sec)

@mulby

mulby commented Feb 9, 2016

Copy link
Copy Markdown

@efischer19 another option is to move the records to an archive table when they are deleted. Something like "submissions_studentitem_deleted".

I'll follow up with an email to the data layer working group.

@efischer19

Copy link
Copy Markdown
Contributor Author

@mulby I went ahead and implemented the archive table approach on #36, it looks like it will handle everything nicely from the user-perspective side of things.

@mulby

mulby commented Feb 9, 2016

Copy link
Copy Markdown

It sounds like there may be some precedent in the ORA code to implement a "tombstone" record combined with an append-only history.

This would also work well for analytics. Not sure how feasible it is to implement in this area of the code, however, I'll leave that decision up to you and @ormsbee

@efischer19 efischer19 closed this Feb 18, 2016
@nsprenkle
nsprenkle deleted the efischer/mangle_key branch November 12, 2021 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants