openstack
/
designate
Code Issues Proposed changes
designate/2c935b03e9ad0ec1b877d6ac4bb...

305 lines
10 KiB
Plaintext
Raw Blame History

{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "b7b14f59_335a322a",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 7
},
"lineNbr": 0,
"author": {
"id": 22623
},
"writtenOn": "2023-03-04T03:10:37Z",
"side": 1,
"message": "Every worker is sending a heartbeat. This means that if you have 5 processes (workers) each one of them will send a separate heartbeat. This gets pretty crazy if you have lets say 20+ processes on a single machine.",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "d6be9ea9_a84488a9",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 7
},
"lineNbr": 0,
"author": {
"id": 32919
},
"writtenOn": "2023-03-12T09:47:11Z",
"side": 1,
"message": "that is current architecture and this PR is not dealing with it. I am handling the scenario where service status is marked wrongly as \u0027UP\u0027 even if service is not as well as deleting stale entries thereby introducing \u0027STOPPED\u0027 state.",
"parentUuid": "b7b14f59_335a322a",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "817cabcc_9dab2ca4",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 7
},
"lineNbr": 0,
"author": {
"id": 22623
},
"writtenOn": "2023-03-12T19:56:31Z",
"side": 1,
"message": "Yep, but this was introduced with this PR, before this it was only done on the parent process.",
"parentUuid": "d6be9ea9_a84488a9",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "dd377d06_f7298ea7",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 7
},
"lineNbr": 0,
"author": {
"id": 32919
},
"writtenOn": "2023-03-15T09:17:44Z",
"side": 1,
"message": "yes, what is your suggestion then ?",
"parentUuid": "817cabcc_9dab2ca4",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "f0c2f782_8dc55899",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 7
},
"lineNbr": 0,
"author": {
"id": 22623
},
"writtenOn": "2023-03-19T21:36:58Z",
"side": 1,
"message": "You can start the heartbeat timer in the init portion of the service. This will prevent it from being started on each fork.\nhttps://github.com/openstack/senlin/blob/master/senlin/common/service.py#L45",
"parentUuid": "dd377d06_f7298ea7",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "75659c4f_f9e36916",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 7
},
"lineNbr": 0,
"author": {
"id": 32919
},
"writtenOn": "2023-03-21T14:42:30Z",
"side": 1,
"message": "I had mentioned issues with this approach earlier https://review.opendev.org/c/openstack/designate/+/866698/comments/8bac2428_6de72227",
"parentUuid": "f0c2f782_8dc55899",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "9d1b1070_2232dc4b",
"filename": "designate/heartbeat_emitter.py",
"patchSetId": 7
},
"lineNbr": 70,
"author": {
"id": 22623
},
"writtenOn": "2023-03-04T03:28:23Z",
"side": 1,
"message": "These statuses should all be defined as constants in designate/common/constants.py",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "e5ffbd22_20ff8823",
"filename": "designate/heartbeat_emitter.py",
"patchSetId": 7
},
"lineNbr": 70,
"author": {
"id": 32919
},
"writtenOn": "2023-03-12T09:47:11Z",
"side": 1,
"message": "ok, changed",
"parentUuid": "9d1b1070_2232dc4b",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "5e9e7fdf_7d98f638",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 278,
"author": {
"id": 22623
},
"writtenOn": "2023-03-04T03:05:54Z",
"side": 1,
"message": "This should probably be two different tasks.",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "0faee02f_83db5a14",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 278,
"author": {
"id": 32919
},
"writtenOn": "2023-03-12T09:47:11Z",
"side": 1,
"message": "ok, updated.\nThe default interval for cleanup of stopped services changed to 48 hours.",
"parentUuid": "5e9e7fdf_7d98f638",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "bada08d5_d65d0e8d",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 310,
"author": {
"id": 22623
},
"writtenOn": "2023-03-04T03:05:54Z",
"side": 1,
"message": "If a service is unavailable for longer than lets say 5min it should be marked as down.",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "92151865_bb53e839",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 310,
"author": {
"id": 32919
},
"writtenOn": "2023-03-12T09:47:11Z",
"side": 1,
"message": "its marked as DOWN.",
"parentUuid": "bada08d5_d65d0e8d",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "992af45f_cad16407",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 318,
"author": {
"id": 22623
},
"writtenOn": "2023-03-04T03:05:54Z",
"side": 1,
"message": "A service down (or not updated) for maybe a week should be removed.",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "88dfa05f_9261841d",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 318,
"author": {
"id": 22623
},
"writtenOn": "2023-03-04T03:29:03Z",
"side": 1,
"message": "How does a status transition from DOWN to STOPPED?",
"parentUuid": "992af45f_cad16407",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "8c805db7_d95d718b",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 318,
"author": {
"id": 32919
},
"writtenOn": "2023-03-12T09:47:11Z",
"side": 1,
"message": "Service stopped is intentionally stopped by user, the down service never goes to stopped state otherwise. \n1. start service, - UP\n2. stop service - STOPPED\n3. start service, if its same service, it will be UP else previous service entry as STOPPED and new entry as UP\n4. Periodic function, checks for services,\n a. if UP is not heartbeated for longer than desired threshold, it will go to DOWN\n b. if STOPPED, that entry will be deleted.",
"parentUuid": "88dfa05f_9261841d",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "ec762044_216d0c69",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 318,
"author": {
"id": 22623
},
"writtenOn": "2023-03-12T19:56:31Z",
"side": 1,
"message": "Sounds reasonable, but in your scenario when something goes down (without properly being marked as STOPPED e.g. service, rabbitmq or database crash) it will never be deleted, since the name is likely randomized in k8s.",
"parentUuid": "8c805db7_d95d718b",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "a918ae4c_f89ed0f6",
"filename": "designate/producer/tasks.py",
"patchSetId": 7
},
"lineNbr": 318,
"author": {
"id": 32919
},
"writtenOn": "2023-03-15T09:17:44Z",
"side": 1,
"message": "yes, service will then be marked as DOWN by next deployment. But there is no way to know DOWN is DOWN in current deployment or previous deployment. So this should be fine, as this is rare case, admin can cleanup db once in while.",
"parentUuid": "ec762044_216d0c69",
"revId": "2c935b03e9ad0ec1b877d6ac4bb9720a0cf830b1",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}
View Git Blame Copy Permalink