You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by "Sijie Guo (JIRA)" <ji...@apache.org> on 2012/05/12 03:17:49 UTC

[jira] [Issue Comment Edited] (BOOKKEEPER-256) Update remote component sequence IDs only for messages from the source region

    [ https://issues.apache.org/jira/browse/BOOKKEEPER-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13273783#comment-13273783 ] 

Sijie Guo edited comment on BOOKKEEPER-256 at 5/12/12 1:17 AM:
---------------------------------------------------------------

just some comments:

1) seems that the line you changed in RegionManager is not used. so my question is how we propagate the seqid from src region to remote components list, the takeRegionMaximum() might not work correctly? if I missed anything, please correct me.

2) so you just improved the behavior of takeRegionMaximum, so it just take the maximum id for source region, right? 

{code}
+    public static void takeRegionSpecificMaximum(MessageSeqId.Builder newIdBuilder, MessageSeqId id1, MessageSeqId id2, ByteString srcRegion) {
+        Map<ByteString, RegionSpecificSeqId> id2Map = MessageIdUtils.inMapForm(id2);
+        for (RegionSpecificSeqId rssid1 : id1.getRemoteComponentsList()) {
+            if(rssid1.getRegion().equals(srcRegion)) {
+                RegionSpecificSeqId rssid2 = id2Map.get(srcRegion);
+                if (rssid2 != null) {
+                    newIdBuilder.addRemoteComponents((rssid1.getSeqId() > rssid2.getSeqId() ? rssid1 : rssid2));
+                } else {
+                    newIdBuilder.addRemoteComponents(rssid1);
+                }
+            } else {
+                newIdBuilder.addRemoteComponents(rssid1);
+            }
+            id2Map.remove(rssid1.getRegion());
+        }
+        for (RegionSpecificSeqId rssid2 : id2Map.values()) {
+            if (rssid2.getRegion().equals(srcRegion)) {
+                newIdBuilder.addRemoteComponents(rssid2);
+            }
+        }
+    }
{code} 

so the id2 is srcRegion seq id, right? If so, we don't need to check region specific of src region in id2.
it could be 

{code}
+        for (RegionSpecificSeqId rssid1 : id1.getRemoteComponentsList()) {
+            if(rssid1.getRegion().equals(srcRegion)) {
                 RegionSpecificSeqId newRssid = rssid1;
                 if (rssid1.getSeqId() < id2.getLocalComponent()) {
                     newRssid = ... // build region specific seq id here.
                 }
                 newIdBuilder.addRemoteComponents(newRssid);
+            } else {
+                newIdBuilder.addRemoteComponents(rssid1);
+            }
+            id2Map.remove(rssid1.getRegion());
+        }
{code}
                
      was (Author: hustlmsp):
    just some comments:

1) seems that the line you changed in RegionManager is not used. so my question is how we propagate the seqid from src region to remote components list, the takeRegionMaximum() might not work correctly? if I missed anything, please correct me.

2) so you just improved the behavior of takeRegionMaximum, so it just take the maximum id for source region, right? 

{quote}
+    public static void takeRegionSpecificMaximum(MessageSeqId.Builder newIdBuilder, MessageSeqId id1, MessageSeqId id2, ByteString srcRegion) {
+        Map<ByteString, RegionSpecificSeqId> id2Map = MessageIdUtils.inMapForm(id2);
+        for (RegionSpecificSeqId rssid1 : id1.getRemoteComponentsList()) {
+            if(rssid1.getRegion().equals(srcRegion)) {
+                RegionSpecificSeqId rssid2 = id2Map.get(srcRegion);
+                if (rssid2 != null) {
+                    newIdBuilder.addRemoteComponents((rssid1.getSeqId() > rssid2.getSeqId() ? rssid1 : rssid2));
+                } else {
+                    newIdBuilder.addRemoteComponents(rssid1);
+                }
+            } else {
+                newIdBuilder.addRemoteComponents(rssid1);
+            }
+            id2Map.remove(rssid1.getRegion());
+        }
+        for (RegionSpecificSeqId rssid2 : id2Map.values()) {
+            if (rssid2.getRegion().equals(srcRegion)) {
+                newIdBuilder.addRemoteComponents(rssid2);
+            }
+        }
+    }
{quote} 

so the id2 is srcRegion seq id, right? If so, we don't need to check region specific of src region in id2.
it could be 

{code}
+        for (RegionSpecificSeqId rssid1 : id1.getRemoteComponentsList()) {
+            if(rssid1.getRegion().equals(srcRegion)) {
                 RegionSpecificSeqId newRssid = rssid1;
                 if (rssid1.getSeqId() < id2.getLocalComponent()) {
                     newRssid = ... // build region specific seq id here.
                 }
                 newIdBuilder.addRemoteComponents(newRssid);
+            } else {
+                newIdBuilder.addRemoteComponents(rssid1);
+            }
+            id2Map.remove(rssid1.getRegion());
+        }
{code}
                  
> Update remote component sequence IDs only for messages from the source region
> -----------------------------------------------------------------------------
>
>                 Key: BOOKKEEPER-256
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-256
>             Project: Bookkeeper
>          Issue Type: Improvement
>          Components: hedwig-server
>    Affects Versions: 4.1.0
>            Reporter: Aniruddha
>            Assignee: Aniruddha
>            Priority: Minor
>             Fix For: 4.1.0
>
>         Attachments: BK-256.patch
>
>
> In the current setup, remote components for a message being persisted don't give any meaningful information about the number of messages the hub has received for that topic from other regions. The provided patch fixes this by updating the remote component of the locally persisted message for a region X only if the message received by the RegionManager originates from region X. 
> Edit - You can take a look at the discussion at http://mail-archives.apache.org/mod_mbox/zookeeper-bookkeeper-dev/201205.mbox/%3cCAOLhyDTEm5=p8eMD8XmVCY_6ktB40RQx6dWWY50ARbAEbdgtsQ@mail.gmail.com%3e for context.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira