Skip to content

ActiveRecord destroy_all short circuits on first failure #950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tanner-rutgers opened this issue Jul 28, 2020 · 1 comment
Open

ActiveRecord destroy_all short circuits on first failure #950

tanner-rutgers opened this issue Jul 28, 2020 · 1 comment

Comments

@tanner-rutgers
Copy link

tanner-rutgers commented Jul 28, 2020

Not sure if this is expected behaviour, but when attempting to use destroy_all on an indexed ActiveRecord model, if any of the records are missing from the index, the first 404 raises an exception and prevents the rest of the records from being removed from the database and/or index, while the record itself that was not in the index is still removed from the db.

Compare this to the behaviour when destroy on a single record that's missing an index, in which case the record is still removed from the db prior to the 404 being raised.

Example

Database records
image

Setting up the index

irb(main):008:0> Document.__elasticsearch__.create_index! force: true
2020-07-28 18:20:08 -0400: DELETE http://shipping-search.railgun:9200/documents [status:404, request:0.003s, query:N/A]
2020-07-28 18:20:08 -0400: < {"error":{"root_cause":[{"type":"index_not_found_exception","reason":"no such index [documents]","resource.type":"index_or_alias","resource.id":"documents","index_uuid":"_na_","index":"documents"}],"type":"index_not_found_exception","reason":"no such index [documents]","resource.type":"index_or_alias","resource.id":"documents","index_uuid":"_na_","index":"documents"},"status":404}
2020-07-28 18:20:08 -0400: [404] {"error":{"root_cause":[{"type":"index_not_found_exception","reason":"no such index [documents]","resource.type":"index_or_alias","resource.id":"documents","index_uuid":"_na_","index":"documents"}],"type":"index_not_found_exception","reason":"no such index [documents]","resource.type":"index_or_alias","resource.id":"documents","index_uuid":"_na_","index":"documents"},"status":404}
2020-07-28 18:20:08 -0400: [!!!] Index does not exist (Elasticsearch::Transport::Transport::Errors::NotFound)
2020-07-28 18:20:08 -0400: HEAD http://shipping-search.railgun:9200/documents [status:404, request:0.002s, query:N/A]
2020-07-28 18:20:08 -0400: <
2020-07-28 18:20:08 -0400: [404]
2020-07-28 18:20:08 -0400: PUT http://shipping-search.railgun:9200/documents [status:200, request:0.084s, query:n/a]
2020-07-28 18:20:08 -0400: > {"settings":{"index":{"number_of_shards":1}},"mappings":{"dynamic":"false","properties":{"url":{"type":"text"},"content":{"type":"text"}}}}
2020-07-28 18:20:08 -0400: < {"acknowledged":true,"shards_acknowledged":true,"index":"documents"}
=> {"acknowledged"=>true, "shards_acknowledged"=>true, "index"=>"documents"}

Importing data

irb(main):009:0> Document.__elasticsearch__.import
2020-07-28 18:20:24 -0400: HEAD http://shipping-search.railgun:9200/documents [status:200, request:0.003s, query:n/a]
2020-07-28 18:20:24 -0400: <
  Document Load (3.7ms)  SELECT `documents`.* FROM `documents` ORDER BY `documents`.`id` ASC LIMIT 1000
2020-07-28 18:20:24 -0400: POST http://shipping-search.railgun:9200/documents/_bulk [status:200, request:0.020s, query:0.013s]
2020-07-28 18:20:24 -0400: > {"index":{"_id":31}}
{"id":31,"url":"http://test1.com","content":"content1","created_at":"2020-07-23T20:20:57.284Z","updated_at":"2020-07-23T20:20:57.284Z"}
{"index":{"_id":33}}
{"id":33,"url":"http://test2.com","content":"content2","created_at":"2020-07-23T20:20:57.284Z","updated_at":"2020-07-23T20:20:57.284Z"}
{"index":{"_id":34}}
{"id":34,"url":"http://test3.com","content":"content3","created_at":"2020-07-23T20:20:57.284Z","updated_at":"2020-07-23T20:20:57.284Z"}
{"index":{"_id":35}}
{"id":35,"url":"http://test4.com","content":"content4","created_at":"2020-07-23T20:20:57.284Z","updated_at":"2020-07-23T20:20:57.284Z"}

2020-07-28 18:20:24 -0400: < {"took":13,"errors":false,"items":[{"index":{"_index":"documents","_type":"_doc","_id":"31","_version":1,"result":"created","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":0,"_primary_term":1,"status":201}},{"index":{"_index":"documents","_type":"_doc","_id":"33","_version":1,"result":"created","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":1,"_primary_term":1,"status":201}},{"index":{"_index":"documents","_type":"_doc","_id":"34","_version":1,"result":"created","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":2,"_primary_term":1,"status":201}},{"index":{"_index":"documents","_type":"_doc","_id":"35","_version":1,"result":"created","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":3,"_primary_term":1,"status":201}}]}
=> 0

Deleting an elasticsearch document for just one of the records

irb(main):010:0> Document.second.__elasticsearch__.delete_document
  Document Load (1.0ms)  SELECT `documents`.* FROM `documents` ORDER BY `documents`.`id` ASC LIMIT 1 OFFSET 1
2020-07-28 18:21:00 -0400: DELETE http://shipping-search.railgun:9200/documents/_doc/33 [status:200, request:0.007s, query:n/a]
2020-07-28 18:21:00 -0400: < {"_index":"documents","_type":"_doc","_id":"33","_version":2,"result":"deleted","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":4,"_primary_term":1}
=> {"_index"=>"documents", "_type"=>"_doc", "_id"=>"33", "_version"=>2, "result"=>"deleted", "_shards"=>{"total"=>2, "successful"=>1, "failed"=>0}, "_seq_no"=>4, "_primary_term"=>1}

destroy_all

irb(main):011:0> Document.destroy_all
  Document Load (1.0ms)  SELECT `documents`.* FROM `documents`
   (0.6ms)  BEGIN
  Document Destroy (0.6ms)  DELETE FROM `documents` WHERE `documents`.`id` = 31
   (2.5ms)  COMMIT
2020-07-28 18:21:09 -0400: DELETE http://shipping-search.railgun:9200/documents/_doc/31 [status:200, request:0.006s, query:n/a]
2020-07-28 18:21:09 -0400: < {"_index":"documents","_type":"_doc","_id":"31","_version":2,"result":"deleted","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":5,"_primary_term":1}
   (0.4ms)  BEGIN
  Document Destroy (0.5ms)  DELETE FROM `documents` WHERE `documents`.`id` = 33
   (1.4ms)  COMMIT
2020-07-28 18:21:09 -0400: DELETE http://shipping-search.railgun:9200/documents/_doc/33 [status:404, request:0.004s, query:N/A]
2020-07-28 18:21:09 -0400: < {"_index":"documents","_type":"_doc","_id":"33","_version":3,"result":"not_found","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":6,"_primary_term":1}
2020-07-28 18:21:09 -0400: [404] {"_index":"documents","_type":"_doc","_id":"33","_version":3,"result":"not_found","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":6,"_primary_term":1}
Not notifying due to an invalid api_key
Traceback (most recent call last):
        1: from (irb):11
Elasticsearch::Transport::Transport::Errors::NotFound ([404] {"_index":"documents","_type":"_doc","_id":"33","_version":3,"result":"not_found","_shards":{"total":2,"successful":1,"failed":0},"_seq_no":6,"_primary_term":1})

Table showing only first record was deleted, second record was deleted, remaining were not
image

Expected behaviour

I would expect destroy_all to either:

a) destroy all records from the db and alert but not prevent deletion if some record indexes were unable to be found
b) roll back prior transactions on failure to delete an index

Actual behaviour

DB is left in an inconsistent state with records up to and including the first failed document deletion deleted, while remaining records are untouched

@stale stale bot added the stale label Sep 30, 2020
@elastic elastic deleted a comment from stale bot Sep 30, 2020
@stale stale bot removed the stale label Sep 30, 2020
@stale
Copy link

stale bot commented Dec 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 4, 2020
@stale stale bot closed this as completed Dec 12, 2020
@picandocodigo picandocodigo reopened this Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants