Skip to content
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

Add warning logs for manifests, siblings, bytes and history #915

Merged
merged 2 commits into from
Jul 16, 2014

Conversation

shino
Copy link
Contributor

@shino shino commented Jul 15, 2014

Add three kinks of warning logs for potentially bloated manifests.
Original issue is #882.

  • many siblings
  • large size
  • long history ({UUID, M} pairs)

Each has default value and enabled, but has kill switch disabled.

- many siblings
- large size
- long history ({UUID, M} pairs)
@shino shino added this to the 1.5.0 milestone Jul 15, 2014
@@ -432,6 +433,40 @@ gc_deleted_while_writing_manifests(Object, Manifests, Bucket, Key, RcPid) ->
UUIDs = riak_cs_manifest_utils:deleted_while_writing(Manifests),
riak_cs_gc:gc_specific_manifests(UUIDs, Object, Bucket, Key, RcPid).

-spec maybe_log_warning(binary(), binary(), riakc_obj:riakc_obj(), [term()]) -> ok.
maybe_log_warning(Bucket, Key, Object, Manifests) ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, the name of this function seems too general. I prefer this be maybe_warn_manifest_size/4 and in riak_cs_manifest.erl .

@kuenishi
Copy link
Contributor

I tested overwriting hundreds times, then I got logs like this per single put:

2014-07-15 16:41:59.286 [warning] <0.4983.0>@riak_cs_utils:maybe_log_warning:465 Long manifest history for key test/md.xml: 451 manifests
2014-07-15 16:41:59.299 [warning] <0.4985.0>@riak_cs_utils:maybe_log_warning:465 Long manifest history for key test/md.xml: 451 manifests

This is maybe because get_manifets is called twice somewhere in put_fsm ... I would like to get just once.

@shino
Copy link
Contributor Author

shino commented Jul 16, 2014

This is maybe because get_manifets is called twice somewhere in put_fsm ... I would like to get just once.

Both riak_cs_get_fsm and riak_cs_put_fsm have riak_cs_manifest_fsm each.
It may be a sign for refactoring 😄 .
I want to defer it after 1.5 release. Thoughts?

- get_manfiests
- manifests_from_riak_object
@shino
Copy link
Contributor Author

shino commented Jul 16, 2014

@kota Addressed your comments except the last one and pushed commit.

Now logs look like these:

11:46:55.124 [warning] Long manifest history (32 manifests) for bucket=<<"test2">> key=<<"\t">>
11:48:55.776 [warning] Many manifest siblings (210 siblings) for bucket=<<"test">> key=<<"mp5">>
11:48:55.776 [warning] Large manifest size (7882640 bytes) for bucket=<<"test">> key=<<"mp5">>

@shino
Copy link
Contributor Author

shino commented Jul 16, 2014

Ouch, I mentioned wrong Kota-san. Sorry again and again.

I wanted to mention @kuenishi 😓

borshop added a commit that referenced this pull request Jul 16, 2014
…rning

Add warning logs for manifests, siblings, bytes and history

Reviewed-by: kuenishi
@shino
Copy link
Contributor Author

shino commented Jul 16, 2014

@borshop merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants