sonic-net / sonic-buildimage

Scripts which perform an installable binary image build for SONiC
Other
717 stars 1.38k forks source link

Duplicated dbid in redis multi-instances causes error #5351

Open BrynXu opened 3 years ago

BrynXu commented 3 years ago

Description Ideally dbid in different redis instances can be duplicated, but due to following code:

const TableNameSeparatorMap TableBase::tableNameSeparatorMap = {
   { APPL_DB,             TABLE_NAME_SEPARATOR_COLON },
   { ASIC_DB,             TABLE_NAME_SEPARATOR_COLON },
   { COUNTERS_DB,         TABLE_NAME_SEPARATOR_COLON },
   { LOGLEVEL_DB,         TABLE_NAME_SEPARATOR_COLON },
   { CONFIG_DB,           TABLE_NAME_SEPARATOR_VBAR  },
   { PFC_WD_DB,           TABLE_NAME_SEPARATOR_COLON },
   { FLEX_COUNTER_DB,     TABLE_NAME_SEPARATOR_COLON },
   { STATE_DB,            TABLE_NAME_SEPARATOR_VBAR  },
   { GB_ASIC_DB,          TABLE_NAME_SEPARATOR_VBAR  },
   { GB_COUNTERS_DB,      TABLE_NAME_SEPARATOR_VBAR  },
   { GB_FLEX_COUNTER_DB,  TABLE_NAME_SEPARATOR_VBAR  },
   { GLOBAL_APP_DB,       TABLE_NAME_SEPARATOR_VBAR  }
};

dup dbid in different instances will cause problem.

Steps to reproduce the issue: 1. 2. 3.

Describe the results you received:

Describe the results you expected:

Additional information you deem important (e.g. issue happens only occasionally):

**Output of `show version`:**

```
(paste your output here)
```

**Attach debug file `sudo generate_dump`:**

```
(paste your output here)
```
qiluo-msft commented 3 years ago

The tableNameSeparatorMap and the constructor TableBase(int dbId, const std::string &tableName) is going to deprecate in future, so it is suggested not to extend the map. Instead the same functionality could be achieved by database_config.json. Could you check?

dzhangalibaba commented 3 years ago

we need the dbid unique to go ahead with current multidb warmboot design.

dzhangalibaba commented 3 years ago

Any case we must use duplicated dbids?

BrynXu commented 3 years ago

not really a case, we found we have to use unique dbid for new db in new redis instance and assign it unique id, @lguohan raised a concern why the dbid is unique for the db in different redis instance. Per my understand, dbids from different redis instances are not relevant, redis allows duplication among them, it is current sonic code introduces the unique id requirement, it is better to indicate this in doc for redis multi-instance developers.