[Starlingx-discuss] Questions about patch upstreaming for e490b5a and 8e7249
Hi Matt, We're looking into the below patches, could you please help to clarify below questions? Thanks much! * e490b5a: CGTS-3239: add process synchronization around segment allocation
From the comments "in upstream they attempted to address this by changing the segmentation allocation to be random so that the probability of 2 processes accessing the same record is lessened but it does not completely eliminate the possibility. In our environment we allocate sequentially so the likelyhood of this occurring is even greater. We allocate sequentially to facilitate predictable network setup order."
Do you think this issue still need to be addressed when STX adopt upstream type driver since (1) the possibility is low and (2) the patch introduce a global lock which may have side effect on performance? * 8e7249: GTS-6926: wsgi: prevent accepting socket without a greenthread
From the comments "This is really a bug (IMHO) in the eventlet code but to test the waters and make sure this works properly I am pushing this to neutron first before deciding whether to make the same/equivalent change at a lower level. Neutron is planning on moving away from the eventlet.wsgi.server so hopefully in the future the API will be run by a proper webserver which won't be susceptible to this type of failure."
By reviewing the code, I think Allain is correct and it make more sense to fix this issue in low level code (maybe in eventlet.wsgi.Server, check the status for available working threads then determine whether to process the request or return 'server busy' error directly; by applying the patch in neutron, the client may get "server unavailable" error). And since eventlet.wsgi.server is planned to be removed by neutron, do you think hold on this fix for now make sense? Thank much! Best Regards, Le, Huifeng
[WIND]<http://www.windriver.com/> From: Le, Huifeng [mailto:huifeng.le@intel.com] Sent: Tuesday, October 23, 2018 3:40 AM To: Peters, Matt Cc: starlingx-discuss@lists.starlingx.io Subject: [Starlingx-discuss] Questions about patch upstreaming for e490b5a and 8e7249 Hi Matt, We're looking into the below patches, could you please help to clarify below questions? Thanks much! * e490b5a: CGTS-3239: add process synchronization around segment allocation
From the comments "in upstream they attempted to address this by changing the segmentation allocation to be random so that the probability of 2 processes accessing the same record is lessened but it does not completely eliminate the possibility. In our environment we allocate sequentially so the likelyhood of this occurring is even greater. We allocate sequentially to facilitate predictable network setup order."
Do you think this issue still need to be addressed when STX adopt upstream type driver since (1) the possibility is low and (2) the patch introduce a global lock which may have side effect on performance? [AL] My opinion is that the upstream method of avoiding the issue is not acceptable and a solution is required. Any code that relies on the low likelihood of a failure occurring rather than coding a solution that works 100% of the time needs to be fixed. * 8e7249: GTS-6926: wsgi: prevent accepting socket without a greenthread
From the comments "This is really a bug (IMHO) in the eventlet code but to test the waters and make sure this works properly I am pushing this to neutron first before deciding whether to make the same/equivalent change at a lower level. Neutron is planning on moving away from the eventlet.wsgi.server so hopefully in the future the API will be run by a proper webserver which won't be susceptible to this type of failure."
By reviewing the code, I think Allain is correct and it make more sense to fix this issue in low level code (maybe in eventlet.wsgi.Server, check the status for available working threads then determine whether to process the request or return 'server busy' error directly; by applying the patch in neutron, the client may get "server unavailable" error). And since eventlet.wsgi.server is planned to be removed by neutron, do you think hold on this fix for now make sense? [AL] Until Neutron adopts a strategy that uses something other than eventlet.wsgi.server we need to apply this patch to avoid the issue that it solves. We should attempt to upstream this fix to Neutron to highlight that this issue exists. Thank much! Best Regards, Le, Huifeng
participants (2)
-
Le, Huifeng
-
Legacy, Allain