[deprecated]86318 move geo index directories and files when database is deleted

[jiangphcn]

Overview

Before change, the geo index directories and files were not deleted when database was deleted. This will cause orphan geo index files and directories. In detail, when database was deleted, the gen_server:cast(?MODULE, {cleanup, DbName}) was called in
https://github.com/cloudant-labs/hastings/blob/master/src/hastings_vacuum.erl#L99. Later, {ok, JsonDDocs} = get_ddocs(DbName) in clean_db/1 was called. Because database was deleted, {'DOWN', Ref, _, _, {error, database_does_not_exist}} message was sent in https://github.com/cloudant-labs/hastings/blob/master/src/hastings_vacuum.erl#L149. Finally, cleanup(DbName, ActiveSigs) was not called in https://github.com/cloudant-labs/hastings/blob/master/src/hastings_vacuum.erl#L127.

This PR is aimed to address above issue and take action against geo index when database is deleted.

The direct approach is to delete geo index files and directoires when database is deleted, i.e. https://github.com/cloudant-labs/hastings/pull/3/files#diff-7ac6be6388d90e131766d8c5824cb226R259

The second approach is to move geo index to directory like /srv/geo/./.recovery/<dbname.ts>/shard/60000000-7fffffff/<dbname.ts>/e66df316792ab411705e2741bba44371 when the corresponding database was deleted and "enable_database_recovery" configuration item is set to true. This allows geo index files to be re-used if database is recovered.
https://github.com/cloudant-labs/hastings/pull/3/files#diff-7ac6be6388d90e131766d8c5824cb226R259

Testing recommendations

make check apps=hastings skip_deps+=couch_epi tests=hastings_delete_db_test_
    Running test function(s):
      hastings_delete_db_test:hastings_delete_db_test_/0
======================== EUnit ========================
Hastings Delete Database Tests
  hastings_delete_db_test:58: should_delete_index_after_deleting_database...[0.377 s] ok
  hastings_delete_db_test:97: should_move_index_after_deleting_database...[0.159 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  [done in 0.884 s]
=======================================================

GitHub issue number

This PR will be the number

Related Pull Requests

N/A

Checklist

• Code is written and works correctly;
• Changes are covered by tests;
• Documentation reflects the changes;

[davisp]

Looks mostly good. I'm questioning the path generation in hastings_util:rename_dir/3 and the hastings_vacuum:cleanup/3 could use some refactoring for simplification.

[davisp]

This logic looks wrong to me. The first thing that comes to mind is if there are repeated directory names. I think we usually do this relative offset using a binary:longest_common_prefix or something.

[davisp]

Big nope to hungarian notation and camel case. Call these selective_clean and full_clean.

[davisp]

Rather than have cleanup do the switching logic between the kind of deletions to perform it'd probably be better to have that logic here. Something like:

Context = couch_util:get_value(context, Options, compaction),
DoRecovery = config:get_boolean("couchdb", "enable_database_recovery", false),
case {Context, DoRecovery} of
    {delete, true} ->
        move_all_indexes(DbName);
    {delete, false} ->
        delete_all_indexes(DbName);
    _ ->
        delete_inactive_indexes(DbName, get_active_sigs())
end.

[jiangphcn]

refactored with your suggestion

[jiangphcn]

@davisp Thanks a lot for your review. I have addressed your comments and also added eunit test. Would you please review when you get time?

[davisp]

One minor nit on the path processing, +1 once that's addressed.

[davisp]

Wouldn't the directory separator be part of the longest common prefix? If not I'd still rather see this as an assertion, something along the lines of:

case Original of
    <<"/", Rest/binary>> -> Rest;
    <<"\", Rest/binary>> -> Rest;
    _ -> Other
end

[jiangphcn]

Under current situation, the RootDir doesn't contain the last directory separator, for example:

 RootDir = /srv/geo_index
 Original = <<"/srv/geo_index/shards/00000000-06666665/user1/db1.1480
073506/20b5ad91cc1334a53700c5da3e835d91">>

I want to remove the "/" in front of shards.

Thanks for your suggestion, I already added assertion to adapt to more situations.

[davisp]

+1