Move checksum file related code to machi_csum_table

kuenishi commented

2015-10-15 02:30:44 +00:00

(Migrated from github.com)

To follow up with trim command, I did some cleanup beforehand. More quickcheck tests could be added, and Unwritten bytes list could be wrapped under checksum table, but I hesitated to do that, to keep each pull request small.

slfritchie commented

2015-10-15 13:17:56 +00:00

(Migrated from github.com)

Hi, Kota, I see three eunit test failures that aren't on master:

  machi_csum_table_test: smoke_test...*failed*
in function machi_checksums:new/1
  called as new("./temp-checksum-dumb-file")
in call from machi_csum_table_test:smoke_test/0 (test/machi_csum_table_test.erl, line 6)
in call from machi_csum_table_test:smoke_test/0
**error:undef

  machi_csum_table_test: close_test...*failed*
in function machi_checksums:new/1
  called as new("./temp-checksum-dumb-file-2")
in call from machi_csum_table_test:close_test/0 (test/machi_csum_table_test.erl, line 18)
in call from machi_csum_table_test:close_test/0
**error:undef

machi_admin_util_test: verify_file_checksums_test_ (module 'machi_admin_util_test')...*failed*
in function machi_admin_util_test:verify_file_checksums_test2/0 (test/machi_admin_util_test.erl, line 53)
**error:{badmatch,
    {error,
        {error,undef,
            [{machi_checksums,split_checksum_list_blob_decode,
                 [<<119,33,0,0,0,0,0,...>>],
                 []},
             {machi_admin_util,verify_file_checksums_common,4,
                 [{file,"src/machi_admin_util.erl"},{line,98}]},
             {machi_admin_util,verify_file_checksums_remote,4,
                 [{file,"src/machi_admin_util.erl"},{line,63}]},
             {machi_admin_util_test,verify_file_checksums_test2,0,
                 [{file,"test/machi_admin_util_test.erl"},{line,53}]},
             {eunit_test,run_testfun,1,[{file,[...]},{line,...}]},
             {eunit_proc,run_test,1,[{file,...},{...}]},
             {eunit_proc,with_timeout,3,[{...}|...]},
             {eunit_proc,handle_test,2,[...]}]}}}

Hi, Kota, I see three eunit test failures that aren't on master: ``` machi_csum_table_test: smoke_test...*failed* in function machi_checksums:new/1 called as new("./temp-checksum-dumb-file") in call from machi_csum_table_test:smoke_test/0 (test/machi_csum_table_test.erl, line 6) in call from machi_csum_table_test:smoke_test/0 **error:undef machi_csum_table_test: close_test...*failed* in function machi_checksums:new/1 called as new("./temp-checksum-dumb-file-2") in call from machi_csum_table_test:close_test/0 (test/machi_csum_table_test.erl, line 18) in call from machi_csum_table_test:close_test/0 **error:undef machi_admin_util_test: verify_file_checksums_test_ (module 'machi_admin_util_test')...*failed* in function machi_admin_util_test:verify_file_checksums_test2/0 (test/machi_admin_util_test.erl, line 53) **error:{badmatch, {error, {error,undef, [{machi_checksums,split_checksum_list_blob_decode, [<<119,33,0,0,0,0,0,...>>], []}, {machi_admin_util,verify_file_checksums_common,4, [{file,"src/machi_admin_util.erl"},{line,98}]}, {machi_admin_util,verify_file_checksums_remote,4, [{file,"src/machi_admin_util.erl"},{line,63}]}, {machi_admin_util_test,verify_file_checksums_test2,0, [{file,"test/machi_admin_util_test.erl"},{line,53}]}, {eunit_test,run_testfun,1,[{file,[...]},{line,...}]}, {eunit_proc,run_test,1,[{file,...},{...}]}, {eunit_proc,with_timeout,3,[{...}|...]}, {eunit_proc,handle_test,2,[...]}]}}} ```

slfritchie commented

2015-10-15 13:24:25 +00:00

(Migrated from github.com)

I like the change in organization. Also, dialyzer output is shorter than the master branch right now, hooray!

jadeallenx commented

2015-10-15 15:51:38 +00:00

(Migrated from github.com)

I like this change too - shouldn't the trim operation be in the file proxy to avoid concurrency problems?

kuenishi commented

2015-10-16 00:48:30 +00:00

(Migrated from github.com)

@slfritchie sorry I was unaware of tests 👿

This code should be updated to strictly find out overlap of written chunks and queried chunks with a strict quickcheck. I want to make sure that any arbitrary set of Offset, Size could be queried from clients, which needs range queries. I changed my mind; csum table is just a table that returns known checksums. Unwritten bytes are managed separately.

@mrallen1 in my brain, trim command is included in the file proxy.

@slfritchie sorry I was unaware of tests :imp: ~~This code should be updated to strictly find out overlap of written chunks and queried chunks with a strict quickcheck. I want to make sure that **any arbitrary set of `Offset, Size` could be queried from clients**, which needs range queries.~~ I changed my mind; csum table is just a table that returns known checksums. Unwritten bytes are managed separately. @mrallen1 in my brain, trim command is included in the file proxy.

kuenishi commented

2015-10-16 03:18:19 +00:00

(Migrated from github.com)

ku/trim-commands is the branch coming up next with a trim command that works. Maybe I'd also have to include whole garbage collection, but I don't have it at once...

[ku/trim-commands](https://github.com/basho/machi/tree/ku/trim-commands) is the branch coming up next with a trim command that works. Maybe I'd also have to include whole garbage collection, but I don't have it at once...

slfritchie commented

2015-10-16 06:09:47 +00:00

(Migrated from github.com)

As the team grows larger, perhaps it will be a good idea to avoid big PRs? Putting the rest of the GC mechanism into later PRs is fine with me.

slfritchie commented

2015-10-16 08:04:54 +00:00

(Migrated from github.com)

Hooray, all tests pass. WIP items are WIP, onward we march.

Move checksum file related code to machi_csum_table #11