*** Qiming has quit IRC | 00:24 | |
*** lvdongbing has joined #senlin | 00:35 | |
*** Qiming has joined #senlin | 01:07 | |
*** yuanying_ has joined #senlin | 01:09 | |
*** yuanying_ has quit IRC | 01:09 | |
*** yuanying has quit IRC | 01:11 | |
*** zhenguo has joined #senlin | 01:13 | |
*** yuanying has joined #senlin | 01:23 | |
*** Yanyanhu has joined #senlin | 01:26 | |
Yanyanhu | hi, Qiming, about the read_committed isolation_level is not allowed in sqlite, I think I can refer to this patch in mistral to make a workaround for unit test | 01:40 |
---|---|---|
Yanyanhu | https://review.openstack.org/#/c/253819/17/mistral/db/sqlalchemy/base.py | 01:40 |
Yanyanhu | but the code could become ugly | 01:41 |
openstackgerrit | Qiming Teng proposed openstack/senlin: Improve logging in functional testings https://review.openstack.org/255273 | 01:44 |
Qiming | yes, it will be pretty ugly | 01:44 |
Qiming | I'm thinking about this | 01:46 |
Qiming | I'm a little worried about the MutableList itself | 01:48 |
*** Liuqing has joined #senlin | 01:53 | |
*** zhenguo has quit IRC | 01:54 | |
openstackgerrit | Qiming Teng proposed openstack/senlin: Fix MutableList implementation https://review.openstack.org/255267 | 01:54 |
*** zhenguo has joined #senlin | 01:55 | |
openstackgerrit | Qiming Teng proposed openstack/senlin: Fix MutableList implementation https://review.openstack.org/255267 | 01:55 |
*** Qiming has quit IRC | 01:56 | |
*** Qiming has joined #senlin | 01:56 | |
*** elynn_ has joined #senlin | 02:00 | |
*** elynn__ has joined #senlin | 02:05 | |
*** elynn_ has quit IRC | 02:05 | |
*** Liuqing has quit IRC | 02:46 | |
Yanyanhu | hi, Qiming, just saw you comment here and gave some reply https://review.openstack.org/#/c/252231/13/senlin/db/sqlalchemy/api.py | 02:54 |
Yanyanhu | so setting the isolation level of transactions to add action dependency means when these transactions are in progress, other transactions to delete dependency can still read and commit the action record | 02:56 |
Qiming | I think this is dangerous | 02:56 |
Qiming | in db theory, you are not supposed to do this asymmetrically | 02:57 |
Yanyanhu | ok. Just feel in our case, add_action_dependency transaction doesn't need so strong consistency protection as del_action_dependency transaction. | 02:58 |
Yanyanhu | so weaken its isolation level | 02:59 |
Qiming | if you want to lower the level, do it to all dependenchy modifications | 03:00 |
Qiming | database doesn't care about your semantics | 03:00 |
Yanyanhu | hmm, but del_action_dependency do need locking read to ensure consistency | 03:01 |
Yanyanhu | so setting read_committed isolation level to all transactions can avoid concurrency issue | 03:01 |
Yanyanhu | s/can/can't | 03:01 |
Qiming | then add_action_dependency is the same | 03:01 |
Qiming | I'm stuck by this logic: | 03:02 |
Qiming | 74 def release(thread, action_id): | 03:02 |
Qiming | 75 '''Callback function that will be passed to GreenThread.link().''' | 03:02 |
Qiming | 76 # Remove action thread from thread list | 03:02 |
Qiming | 77 self.workers.pop(action_id) | 03:02 |
Qiming | 78 action = action_mod.Action.load(self.db_session, action_id) | 03:02 |
Qiming | 79 # This is for actions with RETRY | 03:02 |
Qiming | 80 if action.status == action.READY: | 03:02 |
Qiming | 81 dispatcher.start_action(action_id=action_id) | 03:02 |
Qiming | in engine/scheduler | 03:02 |
Yanyanhu | there are some errors here? | 03:03 |
Qiming | InvalidRequestError: This session is in 'prepared' state; no further SQL can be emitted within this transaction. | 03:03 |
Qiming | that is the error message | 03:04 |
Qiming | but the real question is | 03:04 |
Qiming | why we are doing a start_action inside a release? | 03:04 |
Qiming | and we are deserializing an action object for this task | 03:04 |
*** Liuqing has joined #senlin | 03:05 | |
Qiming | anyway, I'm commenting out line 78-81 | 03:05 |
Yanyanhu | what you mean by 'doing a start_action inside a release'? I think it's for action retry cases | 03:06 |
Yanyanhu | line 81 will send a msg to scheduler to make it schedule this action again | 03:06 |
Qiming | we already have a scheduler retry | 03:07 |
Qiming | when the action is READY, it will be eventually rescheduled | 03:07 |
Yanyanhu | but I think this is the code make it happen? | 03:08 |
Qiming | we are currently creating a race condition inside the scheduler | 03:08 |
Qiming | no ... | 03:08 |
Qiming | it is wrong | 03:08 |
Yanyanhu | since currently, an action will be scheduled at the firsttime it is dispatched | 03:09 |
Qiming | but it is not supposed to do a start inside a release | 03:09 |
Yanyanhu | oh, you mean this logic should be put into other place | 03:09 |
Qiming | 88 action = db_api.action_acquire_1st_ready(self.db_session, | 03:09 |
Qiming | 89 worker_id, | 03:09 |
Qiming | 90 timestamp) | 03:09 |
Qiming | we will be relying on this to do real scheduling | 03:10 |
Yanyanhu | sure | 03:10 |
Yanyanhu | this is the right way | 03:10 |
Yanyanhu | but in as of now, we haven't used it yet | 03:11 |
Qiming | the reschedule() function should do this, it is currently a useless function | 03:11 |
Qiming | it is doing nothing more than a sleep() | 03:12 |
Yanyanhu | yes, it has never been used yet | 03:12 |
Qiming | we are doing things wrong, for the fork-join case | 03:13 |
Qiming | the line 78-81 is creating a race condition | 03:13 |
Qiming | however, we are doing busy-waiting in _wait_for_dependents() | 03:13 |
Qiming | there is no retry logic there | 03:14 |
Yanyanhu | yes | 03:14 |
Yanyanhu | it is not a RESCHEDULE | 03:14 |
Yanyanhu | just a wait now | 03:14 |
Qiming | yes, so there are conflicts | 03:15 |
Qiming | I'm commenting out those logics and doing functional test again | 03:15 |
Qiming | so far so good | 03:15 |
Yanyanhu | conflicts between this wait and the retry logic in line79-81? | 03:15 |
Qiming | yes | 03:15 |
Yanyanhu | yes, sure, since logic 79-81 will only take effect when an action TERMINATED with ready status | 03:16 |
Qiming | let's make RETRY logic a little bit smarter | 03:16 |
Yanyanhu | but in current code, it won't happen I think | 03:16 |
Yanyanhu | sure | 03:16 |
Yanyanhu | this is the right thing to do | 03:16 |
Qiming | let's see what happens | 03:16 |
Yanyanhu | ok | 03:16 |
Qiming | there are still failures, maybe caused by other defects | 03:17 |
Yanyanhu | about the isolation_level issue, I think we don't need to lock the record when starting add_action_dependency transaction | 03:17 |
Qiming | still investigating | 03:17 |
Qiming | don't get you | 03:18 |
Yanyanhu | I mean if a del_action_dependency transaction read and commit to a record that has been read by another add_action_dependency transaction, no error will happen | 03:19 |
Qiming | what do you mean by lock? | 03:19 |
*** yuanying has quit IRC | 03:19 | |
Qiming | I think these are separate issues at different layers | 03:20 |
Yanyanhu | lock means something like SELECT ... LOCK IN SHARE MODE | 03:20 |
Yanyanhu | just locking read | 03:20 |
Qiming | faint | 03:20 |
Qiming | I am not reading the SQL statement | 03:20 |
Qiming | you are introducing another layer of logic into the discussion | 03:21 |
Yanyanhu | oh | 03:21 |
Yanyanhu | sure, this is not about the reschedule issue | 03:21 |
*** Qiming has quit IRC | 03:28 | |
*** Qiming has joined #senlin | 03:29 | |
Qiming | Yanyanhu, can you rebase https://review.openstack.org/#/c/255077/ on to master? | 03:30 |
Qiming | it doesn't make sense to base it on db sync | 03:31 |
Qiming | and also the webhook rework one, onto master | 03:32 |
*** yuanying has joined #senlin | 03:37 | |
*** xuhaiwei has quit IRC | 03:38 | |
*** yuanying has quit IRC | 04:02 | |
*** yuanying has joined #senlin | 04:06 | |
*** elynn__ has quit IRC | 04:12 | |
Yanyanhu | hi, Qiming, will remove the dependency of this patch https://review.openstack.org/255077 | 04:18 |
openstackgerrit | Merged openstack/senlin: Improve logging in functional testings https://review.openstack.org/255273 | 04:34 |
openstackgerrit | Yanyan Hu proposed openstack/senlin: Pework functional tests of cluster_list and node_list https://review.openstack.org/255077 | 04:37 |
openstackgerrit | Qiming Teng proposed openstack/senlin: Remove default handling for 'RETRY' actions https://review.openstack.org/255668 | 04:38 |
*** xuhaiwei has joined #senlin | 04:40 | |
*** elynn___ has joined #senlin | 04:44 | |
openstackgerrit | Merged openstack/senlin: Shorten the event ID in log output for reability https://review.openstack.org/255276 | 04:45 |
Qiming | hi, guys | 04:49 |
openstackgerrit | Merged openstack/senlin: Pework functional tests of cluster_list and node_list https://review.openstack.org/255077 | 04:49 |
Qiming | Senlin API doc is online! | 04:49 |
Qiming | http://developer.openstack.org/api-ref-clustering-v1.html | 04:49 |
elynn___ | cool!! | 04:50 |
openstackgerrit | Qiming Teng proposed openstack/senlin: Fix MutableList implementation https://review.openstack.org/255267 | 04:51 |
Yanyanhu | great! | 04:53 |
*** pratikmallya has joined #senlin | 05:00 | |
openstackgerrit | Qiming Teng proposed openstack/senlin: Remove local copy of api doc https://review.openstack.org/255674 | 05:03 |
*** Qiming has quit IRC | 05:08 | |
xuhaiwei | finally it comes | 05:30 |
*** Qiming has joined #senlin | 05:31 | |
lixinhui | Great!! | 05:40 |
openstackgerrit | Merged openstack/senlin: Remove local copy of api doc https://review.openstack.org/255674 | 05:41 |
*** Liuqing has quit IRC | 05:43 | |
*** Liuqing has joined #senlin | 05:44 | |
*** Yanyanhu has quit IRC | 05:46 | |
*** Yanyanhu has joined #senlin | 05:47 | |
openstackgerrit | Qiming Teng proposed openstack/senlin: Fix message in developer doc linking to API https://review.openstack.org/255681 | 05:56 |
*** pratikmallya has quit IRC | 06:03 | |
*** Qiming has quit IRC | 06:08 | |
*** Yanyanhu has quit IRC | 06:43 | |
*** lixinhui_ has joined #senlin | 07:16 | |
*** lixinhui has quit IRC | 07:17 | |
*** shu-mutou has quit IRC | 07:22 | |
*** gongysh_ has joined #senlin | 07:23 | |
openstackgerrit | Merged openstack/senlin: Fix message in developer doc linking to API https://review.openstack.org/255681 | 07:34 |
openstackgerrit | lvdongbing proposed openstack/senlin: Revise 'Heat' to 'Senlin' in doc/docbkx/README.rst https://review.openstack.org/255703 | 07:37 |
openstackgerrit | Merged openstack/senlin: Updated from global requirements https://review.openstack.org/255564 | 07:55 |
*** lixinhui_ has quit IRC | 08:23 | |
openstackgerrit | xu-haiwei proposed openstack/python-senlinclient: Add test case for v1/shell.py part2 https://review.openstack.org/251798 | 08:26 |
*** yuanying has quit IRC | 09:10 | |
openstackgerrit | junxu proposed openstack/senlin: Add block_device_mapping_v2 support for 'os.nova.server' profile. https://review.openstack.org/255755 | 09:22 |
*** openstackgerrit has quit IRC | 09:32 | |
*** openstackgerrit has joined #senlin | 09:33 | |
*** lvdongbing has quit IRC | 09:49 | |
*** zhenguo has quit IRC | 10:06 | |
*** Liuqing has quit IRC | 10:19 | |
*** lixinhui has joined #senlin | 10:25 | |
*** elynn___ has quit IRC | 10:29 | |
*** Liuqing has joined #senlin | 10:42 | |
*** elynn___ has joined #senlin | 12:34 | |
*** elynn___ has quit IRC | 12:36 | |
*** elynn has joined #senlin | 12:36 | |
*** openstackgerrit has quit IRC | 12:47 | |
*** openstackgerrit has joined #senlin | 12:48 | |
*** Qiming has joined #senlin | 13:03 | |
*** pratikmallya has joined #senlin | 13:21 | |
openstackgerrit | Merged openstack/senlin: Revise 'Heat' to 'Senlin' in doc/docbkx/README.rst https://review.openstack.org/255703 | 13:31 |
*** openstackstatus has quit IRC | 13:57 | |
openstackgerrit | Merged openstack/senlin: Make webhook-trigger return a location header https://review.openstack.org/255142 | 14:01 |
openstackgerrit | Merged openstack/senlin: Remove default handling for 'RETRY' actions https://review.openstack.org/255668 | 14:28 |
*** lixinhui has quit IRC | 14:34 | |
openstackgerrit | Qiming Teng proposed openstack/senlin: Fix action status setting logic https://review.openstack.org/255925 | 15:05 |
*** pratikmallya has quit IRC | 15:19 | |
openstackgerrit | Qiming Teng proposed openstack/senlin: Fix MutableList implementation https://review.openstack.org/255267 | 15:19 |
*** elynn has quit IRC | 15:23 | |
*** pratikmallya has joined #senlin | 15:54 | |
*** pratikma_ has joined #senlin | 15:57 | |
*** pratikmallya has quit IRC | 16:00 | |
*** Qiming has quit IRC | 16:03 | |
*** openstackstatus has joined #senlin | 17:38 | |
*** ChanServ sets mode: +v openstackstatus | 17:38 | |
openstackgerrit | Liuqing Jing proposed openstack/senlin: Fix typo https://review.openstack.org/256014 | 17:45 |
*** Liuqing has quit IRC | 17:51 | |
*** pratikma_ has quit IRC | 17:57 | |
*** pratikmallya has joined #senlin | 18:41 | |
*** pratikmallya has quit IRC | 18:42 | |
*** pratikmallya has joined #senlin | 18:43 | |
*** pratikma_ has joined #senlin | 21:32 | |
*** pratikmallya has quit IRC | 21:33 | |
*** pratikmallya has joined #senlin | 21:34 | |
*** pratikma_ has quit IRC | 21:37 | |
*** openstackstatus has quit IRC | 23:01 | |
*** pratikmallya has quit IRC | 23:03 | |
*** yuanying has joined #senlin | 23:18 | |
*** Qiming has joined #senlin | 23:35 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!