ejabberd development
  1. ejabberd development
  2. EJAB-1434

Pubsub nodes max_items based on item persistence

    Details

      Description

      In mod_pubsub:publish_item, in case of pubsub#persist_items = 0, max items should be equal to 0.

      1. EJAB_1434_21x.patch
        0.9 kB
        Karim Gemayel
      2. EJAB_1434_30.patch
        0.9 kB
        Karim Gemayel

        Activity

        Hide
        Christophe Romain
        added a comment -

        the max-items should not be fixed, if persist_items is false, we just write nothing into the item database whatever max_items is.
        to follow the spec, we added the handling of last_item_cache.

        A service MAY cache the last item published to a node (even if the "persistent-items" option is set to false); if it does default "cache-last-item" to true, it SHOULD send the last published item (or notification thereof) to subscribed entities based on configuration of the "send_last_published_item" field.

        see set_cached_item function, we use mnesia pubsub_last_item table if cache_last_item is true

        Show
        Christophe Romain
        added a comment - the max-items should not be fixed, if persist_items is false, we just write nothing into the item database whatever max_items is. to follow the spec, we added the handling of last_item_cache. A service MAY cache the last item published to a node (even if the "persistent-items" option is set to false); if it does default "cache-last-item" to true, it SHOULD send the last published item (or notification thereof) to subscribed entities based on configuration of the "send_last_published_item" field. see set_cached_item function, we use mnesia pubsub_last_item table if cache_last_item is true
        Hide
        Daniel Drake
        added a comment -

        This patch caused a regression for me - see EJAB-1533 - the data that I publish gets lost by the server. But it could be due to misconfigured ejabberd or bad message - I'm not quite grasping how this is supposed to work.

        One thing stands out in this patch: max_items explicitly does check for the persist_items case:

        max_items(Host, Options) ->
        case get_option(Options, persist_items) of
        true ->
        case get_option(Options, max_items) of
        false -> unlimited;
        Result when (Result < 0) -> 0;
        Result -> Result
        end;
        false ->
        ...

        So it is not clear to me that removing the call to it is definitely correct - if something is wrong with the logic of max_items in the non-persist case, shouldn't max_items be fixed instead?

        (Sorry if my lack of knowledge of XMPP and erlang is shining through - I'm learning as I go)

        Show
        Daniel Drake
        added a comment - This patch caused a regression for me - see EJAB-1533 - the data that I publish gets lost by the server. But it could be due to misconfigured ejabberd or bad message - I'm not quite grasping how this is supposed to work. One thing stands out in this patch: max_items explicitly does check for the persist_items case: max_items(Host, Options) -> case get_option(Options, persist_items) of true -> case get_option(Options, max_items) of false -> unlimited; Result when (Result < 0) -> 0; Result -> Result end; false -> ... So it is not clear to me that removing the call to it is definitely correct - if something is wrong with the logic of max_items in the non-persist case, shouldn't max_items be fixed instead? (Sorry if my lack of knowledge of XMPP and erlang is shining through - I'm learning as I go)
        Hide
        Karim Gemayel (Inactive)
        added a comment -

        Attached patches for 2.1.x and 3.0.

        Show
        Karim Gemayel (Inactive)
        added a comment - Attached patches for 2.1.x and 3.0.

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since last comment:
              2 years, 8 weeks, 4 days ago

              Issue deployment