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

how I kown the server publish message (use qos1) is success #85

Closed
caoqingguang opened this issue Mar 2, 2018 · 10 comments · May be fixed by #87
Closed

how I kown the server publish message (use qos1) is success #85

caoqingguang opened this issue Mar 2, 2018 · 10 comments · May be fixed by #87

Comments

@caoqingguang
Copy link

caoqingguang commented Mar 2, 2018

when I publish message1 use method
MqttEndpoint publish(topic, Buffer payload, qosLevel, isDup, isRetain);

and i can get messageId by method
int lastMessageId()

and i kown one message is success when i get

client.publishAcknowledgeHandler(id->{
   String msg = getMailBox(client).getMsg(id);
   System.out.println("success:"+id+", msg:"+msg);
});

but in multi thread , the messageId maybe not the real messageId, when an other thread send message2 at same time

so if i want get real messageId for message1, i must use lock, is that?

@Sammers21
Copy link
Contributor

Sammers21 commented Mar 2, 2018

I see the problem is that we are not sure which message id will be returned by MqttEndpoint#lastMessageId in some cases it may return just sent messageId, in some cases may not.

I think we should have one more publish method with a handler, called when a message is sent with a message id. We have a similar method in MqttClient.

@caoqingguang
Copy link
Author

caoqingguang commented Mar 2, 2018

@Sammers21
Thank you for answering my question, I will pull the latest code

and I suggest add 2 methods at interface MqttEndpoint.java
so don't need another map to save the client and context,
User can define any context if they want , and get context at anywhere when they have a ref of
MqttEndpoint .

<T> MqttEndpoint  setContext(T context);

<T>  T  getContext();

@Sammers21
Copy link
Contributor

Sammers21 commented Mar 2, 2018

@caoqingguang, sorry but I did not get it. So, why it can be useful? Can you explain?

You also welcome to submit a PR. We will see and discuss then.

@ppatierno
Copy link
Member

@caoqingguang what's the client context you are talking about ? I think that PR #86 from @Sammers21 can help you to solve your initial problem.

@caoqingguang
Copy link
Author

@Sammers21 @ppatierno
those methods is another suggestion . I can save the remote client status use the client context, ex: the percentage of success message, the average time from send message to receive ack , client connect time, and all message num and so on.

@caoqingguang
Copy link
Author

caoqingguang commented Mar 3, 2018

@Sammers21 @ppatierno
thanks , the PR #86 is ok. and I commit PR #87 that is similar with PR #86
if the message is timeout as not receive ack for a long time. ,I will send the message again with the same messageId.
code like

int messageId=requireMessageId();
publishWithId(message, messageId);

//not receive ack for a long time,then retry
publishWithId(message,messageId);

same message with same messageId,

I suggest those methods can save the mapping (messageId->message)

Map<Integer,String> mailBox=new HashMap<>();
client.setContext(mailBox);

int messageId=requireMessageId();
client.publishWithId(message, messageId);

//not receive ack for a long time,then retry
client.publishWithId(message,messageId);

//when message ack
String message=client.getContext().get(messageId);

@Sammers21
Copy link
Contributor

Sammers21 commented Mar 3, 2018

@caoqingguang, I think that sending a message with id generated by hands might be useful, but it can lead to some issues when using publish with autogenerated message id along with handly generated one. So if we would have such opportunity, it should be used with caution by vertx-mqtt-server users.

And if we would look at a situation when you need to resend a message, why it is a problem to use different from original message Id(autogenerated for example)? It would be simpler and you would have fewer issues with such approach.

If you have a different from the ticket topic suggestions, feel free to create a ticket for each of them. We will discuss.

@caoqingguang
Copy link
Author

caoqingguang commented Mar 4, 2018

@Sammers21
ok, for example, I have two messages to send, each one sent 3 times that first send 1 time and retry 2 times on bad net quality.

case1 same message with diffrent messageId

messageId and message mapping like the table in condition

messageId message description
id1 msg1 first send
id2 msg1 retry send
id3 msg1 retry send again
id4 msg2 first send
id5 msg2 retry send
id6 msg2 retry send again

when receive ack with messageId(id2) from remote client. then i kown the msg1 is send success, and i delete the mapping (id2->msg1), but (id1->msg1) and (id3->msg1) still exists,so i must do more works to delete those mappings.

case2 same message with same messageId

messageId and message mapping like the table in condition

messageId message description
id1 msg1 first send,retry send,retry send again
id4 msg2 first send,retry send,retry send again

when receive ack with messageId(id1) from remote client. then i kown the msg1 is send success, and i only need to delete the mapping (id1->msg1).

when to retry again

I will check the mapping table at future time,and resend the message if it exists in the table.
if i use case1(same message with diffent messageId),i will add more mapping (idN->msg1) then wait remote client ack idN to sure msg1 is success.
and if use case2(same message with same messageId),i don't need do more working beacuse i still wait remote client ack Id1.

@ppatierno
Copy link
Member

@caoqingguang why did you close the issue ? did you solve it ?

@caoqingguang
Copy link
Author

caoqingguang commented Mar 10, 2018

@ppatierno
no,I use the code #PR87 like this

    class StrMsg{
          private int msgId;
          private String msg;
          private long sendTime=System.currentMs();
          private int sendNum;
     }

    Map<MqttEndpoint,Map<Integer,StrMsg>>   clientMailboxMap=...;

    String msg=...;
    int msgId=client.requireMessageId();
     StrMsg  msgObj=new StrMsg(msgId,msg);
    clientMailboxMap.get(client).put(msgId,msgObj);
    client.publishWithId(msg,msgId);

    // when receive ack then
    StrMsg msg=clientMailboxMap.get(client).remove(msgId);
    //todo then tell other business the message is send ok

    //and there had a timer check the clientMailboxMap
    //for each StrMsg do like 
    //1. if  the msg sendNum<3 then add it  and resend the msg use the same msgId,   and
    //2. if now()-sendTime>12s  then remove it  ,and tell the other business the message is send fail.

    //when the remote client reconnect at future time
    // i will tell the other business,  then will triger the message resend again.

I will try run this code on server which has 1 millon users.
if the test is ok, then use it in prod env.

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

Successfully merging a pull request may close this issue.

3 participants