Re: [devel] [PATCH 1/1] log: fix log agent never remove log-client in list if server return BAD_HANDLE [#2910]

2018-08-09 Thread Vu Minh Nguyen
Ack from me.

Thanks, Vu

> -Original Message-
> From: Canh Van Truong 
> Sent: Thursday, August 9, 2018 4:08 PM
> To: 'Lennart Lund' ; 'Vu Minh Nguyen'
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1/1] log: fix log agent never remove log-client in
list if
> server return BAD_HANDLE [#2910]
> 
> Hi Lennart,
> 
> The patch just want to clean up the client in agent in case BAD_HANDLE. I
> attach the new update.
> If you have any comments please let me know.
> 
> Thanks
> Canh
> 
> 
> -Original Message-
> From: Lennart Lund 
> Sent: Tuesday, August 7, 2018 7:33 PM
> To: Canh Van Truong ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: RE: [PATCH 1/1] log: fix log agent never remove log-client in
list
> if server return BAD_HANDLE [#2910]
> 
> Hi Canh,
> 
> Se my comment below.
> 
> I checked the server code and all evt procedures returns BAD HANDLE if
there
> is no client corresponding with the request except for stream open request
> that never return BAD HANDLE. This seems to have something to do with
> headless recovery handling but also this event handler should return BAD
> HANDLE if no valid client and no recovery is ongoing.
> I think this should be investigated
> 
> Regards
> Lennart
> 
> > -Original Message-
> > From: Canh Van Truong 
> > Sent: den 6 augusti 2018 10:37
> > To: Lennart Lund ; Vu Minh Nguyen
> > 
> > Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> > 
> > Subject: [PATCH 1/1] log: fix log agent never remove log-client in list
if
> server
> > return BAD_HANDLE [#2910]
> >
> > Log agent get BAD_HANDLE in some API from log server. The client has
> > already
> > gone away in log server, but it still in the list in agent (lib). it
need
> to be
> > deallocated and removed from list in agent.
> >
> > The patch updates to remove in log API finalize
> > ---
> >  src/log/agent/lga_agent.cc | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/log/agent/lga_agent.cc b/src/log/agent/lga_agent.cc
> > index 2bbc28abd..284876360 100644
> > --- a/src/log/agent/lga_agent.cc
> > +++ b/src/log/agent/lga_agent.cc
> > @@ -567,6 +567,9 @@ SaAisErrorT LogAgent::SendFinalizeMsg(uint32_t
> > client_id) {
> >  ais_rc = SA_AIS_ERR_NO_RESOURCES;
> >}
> >
> [Lennart] This is done in the wrong function. This function is for sending
a
> message and has nothing to do with the logic for handling different return
> codes from the log server. This function shall return whatever return code
> that is coming from the server. Doing fixes like this creates incorrect
> dependencies which makes code hard to read and maintain in the future.
> 
> The following code in LogAgent::saLogFinalize():
>   // from the server end returned before deleting the local records.
>   ais_rc = SendFinalizeMsg(client->GetClientId());
>   if (ais_rc == SA_AIS_OK) {
> 
> Can be changed to:
>   // from the server end returned before deleting the local records.
>   ais_rc = SendFinalizeMsg(client->GetClientId());
>   if ((ais_rc == SA_AIS_OK) || (ais_rc == SA_AIS_ERR_BAD_HANDLE)) {
> 
> > +  // Log client already gone away in log server.
> > +  if (ais_rc == SA_AIS_ERR_BAD_HANDLE) ais_rc = SA_AIS_OK;
> > +
> >return ais_rc;
> >  }
> >
> > --
> > 2.15.1



--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 1/1] log: fix log agent never remove log-client in list if server return BAD_HANDLE [#2910]

2018-08-09 Thread Lennart Lund
Hi Canh,

Ack

Thanks
Lennart

> -Original Message-
> From: Canh Van Truong 
> Sent: den 9 augusti 2018 11:08
> To: Lennart Lund ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 1/1] log: fix log agent never remove log-client in list if
> server return BAD_HANDLE [#2910]
> 
> Hi Lennart,
> 
> The patch just want to clean up the client in agent in case BAD_HANDLE. I
> attach the new update.
> If you have any comments please let me know.
> 
> Thanks
> Canh
> 
> 
> -Original Message-
> From: Lennart Lund 
> Sent: Tuesday, August 7, 2018 7:33 PM
> To: Canh Van Truong ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: RE: [PATCH 1/1] log: fix log agent never remove log-client in list
> if server return BAD_HANDLE [#2910]
> 
> Hi Canh,
> 
> Se my comment below.
> 
> I checked the server code and all evt procedures returns BAD HANDLE if
> there
> is no client corresponding with the request except for stream open request
> that never return BAD HANDLE. This seems to have something to do with
> headless recovery handling but also this event handler should return BAD
> HANDLE if no valid client and no recovery is ongoing.
> I think this should be investigated
> 
> Regards
> Lennart
> 
> > -Original Message-
> > From: Canh Van Truong 
> > Sent: den 6 augusti 2018 10:37
> > To: Lennart Lund ; Vu Minh Nguyen
> > 
> > Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> > 
> > Subject: [PATCH 1/1] log: fix log agent never remove log-client in list if
> server
> > return BAD_HANDLE [#2910]
> >
> > Log agent get BAD_HANDLE in some API from log server. The client has
> > already
> > gone away in log server, but it still in the list in agent (lib). it need
> to be
> > deallocated and removed from list in agent.
> >
> > The patch updates to remove in log API finalize
> > ---
> >  src/log/agent/lga_agent.cc | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/log/agent/lga_agent.cc b/src/log/agent/lga_agent.cc
> > index 2bbc28abd..284876360 100644
> > --- a/src/log/agent/lga_agent.cc
> > +++ b/src/log/agent/lga_agent.cc
> > @@ -567,6 +567,9 @@ SaAisErrorT LogAgent::SendFinalizeMsg(uint32_t
> > client_id) {
> >  ais_rc = SA_AIS_ERR_NO_RESOURCES;
> >}
> >
> [Lennart] This is done in the wrong function. This function is for sending a
> message and has nothing to do with the logic for handling different return
> codes from the log server. This function shall return whatever return code
> that is coming from the server. Doing fixes like this creates incorrect
> dependencies which makes code hard to read and maintain in the future.
> 
> The following code in LogAgent::saLogFinalize():
>   // from the server end returned before deleting the local records.
>   ais_rc = SendFinalizeMsg(client->GetClientId());
>   if (ais_rc == SA_AIS_OK) {
> 
> Can be changed to:
>   // from the server end returned before deleting the local records.
>   ais_rc = SendFinalizeMsg(client->GetClientId());
>   if ((ais_rc == SA_AIS_OK) || (ais_rc == SA_AIS_ERR_BAD_HANDLE)) {
> 
> > +  // Log client already gone away in log server.
> > +  if (ais_rc == SA_AIS_ERR_BAD_HANDLE) ais_rc = SA_AIS_OK;
> > +
> >return ais_rc;
> >  }
> >
> > --
> > 2.15.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 1/1] log: fix log agent never remove log-client in list if server return BAD_HANDLE [#2910]

2018-08-09 Thread Canh Van Truong
Hi Lennart,

The patch just want to clean up the client in agent in case BAD_HANDLE. I
attach the new update.
If you have any comments please let me know.

Thanks
Canh


-Original Message-
From: Lennart Lund  
Sent: Tuesday, August 7, 2018 7:33 PM
To: Canh Van Truong ; Vu Minh Nguyen

Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong

Subject: RE: [PATCH 1/1] log: fix log agent never remove log-client in list
if server return BAD_HANDLE [#2910]

Hi Canh,

Se my comment below.

I checked the server code and all evt procedures returns BAD HANDLE if there
is no client corresponding with the request except for stream open request
that never return BAD HANDLE. This seems to have something to do with
headless recovery handling but also this event handler should return BAD
HANDLE if no valid client and no recovery is ongoing.
I think this should be investigated

Regards
Lennart

> -Original Message-
> From: Canh Van Truong 
> Sent: den 6 augusti 2018 10:37
> To: Lennart Lund ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: [PATCH 1/1] log: fix log agent never remove log-client in list if
server
> return BAD_HANDLE [#2910]
> 
> Log agent get BAD_HANDLE in some API from log server. The client has
> already
> gone away in log server, but it still in the list in agent (lib). it need
to be
> deallocated and removed from list in agent.
> 
> The patch updates to remove in log API finalize
> ---
>  src/log/agent/lga_agent.cc | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/log/agent/lga_agent.cc b/src/log/agent/lga_agent.cc
> index 2bbc28abd..284876360 100644
> --- a/src/log/agent/lga_agent.cc
> +++ b/src/log/agent/lga_agent.cc
> @@ -567,6 +567,9 @@ SaAisErrorT LogAgent::SendFinalizeMsg(uint32_t
> client_id) {
>  ais_rc = SA_AIS_ERR_NO_RESOURCES;
>}
> 
[Lennart] This is done in the wrong function. This function is for sending a
message and has nothing to do with the logic for handling different return
codes from the log server. This function shall return whatever return code
that is coming from the server. Doing fixes like this creates incorrect
dependencies which makes code hard to read and maintain in the future.

The following code in LogAgent::saLogFinalize():
  // from the server end returned before deleting the local records.
  ais_rc = SendFinalizeMsg(client->GetClientId());
  if (ais_rc == SA_AIS_OK) {

Can be changed to:
  // from the server end returned before deleting the local records.
  ais_rc = SendFinalizeMsg(client->GetClientId());
  if ((ais_rc == SA_AIS_OK) || (ais_rc == SA_AIS_ERR_BAD_HANDLE)) {

> +  // Log client already gone away in log server.
> +  if (ais_rc == SA_AIS_ERR_BAD_HANDLE) ais_rc = SA_AIS_OK;
> +
>return ais_rc;
>  }
> 
> --
> 2.15.1



fix_2910.patch
Description: Binary data
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 1/1] log: fix log agent never remove log-client in list if server return BAD_HANDLE [#2910]

2018-08-07 Thread Lennart Lund
Hi Canh,

Se my comment below.

I checked the server code and all evt procedures returns BAD HANDLE if there is 
no client corresponding with the request except for stream open request that 
never return BAD HANDLE. This seems to have something to do with headless 
recovery handling but also this event handler should return BAD HANDLE if no 
valid client and no recovery is ongoing.
I think this should be investigated

Regards
Lennart

> -Original Message-
> From: Canh Van Truong 
> Sent: den 6 augusti 2018 10:37
> To: Lennart Lund ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: [PATCH 1/1] log: fix log agent never remove log-client in list if 
> server
> return BAD_HANDLE [#2910]
> 
> Log agent get BAD_HANDLE in some API from log server. The client has
> already
> gone away in log server, but it still in the list in agent (lib). it need to 
> be
> deallocated and removed from list in agent.
> 
> The patch updates to remove in log API finalize
> ---
>  src/log/agent/lga_agent.cc | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/log/agent/lga_agent.cc b/src/log/agent/lga_agent.cc
> index 2bbc28abd..284876360 100644
> --- a/src/log/agent/lga_agent.cc
> +++ b/src/log/agent/lga_agent.cc
> @@ -567,6 +567,9 @@ SaAisErrorT LogAgent::SendFinalizeMsg(uint32_t
> client_id) {
>  ais_rc = SA_AIS_ERR_NO_RESOURCES;
>}
> 
[Lennart] This is done in the wrong function. This function is for sending a 
message and has nothing to do with the logic for handling different return 
codes from the log server. This function shall return whatever return code that 
is coming from the server. Doing fixes like this creates incorrect dependencies 
which makes code hard to read and maintain in the future.

The following code in LogAgent::saLogFinalize():
  // from the server end returned before deleting the local records.
  ais_rc = SendFinalizeMsg(client->GetClientId());
  if (ais_rc == SA_AIS_OK) {

Can be changed to:
  // from the server end returned before deleting the local records.
  ais_rc = SendFinalizeMsg(client->GetClientId());
  if ((ais_rc == SA_AIS_OK) || (ais_rc == SA_AIS_ERR_BAD_HANDLE)) {

> +  // Log client already gone away in log server.
> +  if (ais_rc == SA_AIS_ERR_BAD_HANDLE) ais_rc = SA_AIS_OK;
> +
>return ais_rc;
>  }
> 
> --
> 2.15.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel