Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MultiDB] SonicV2Connector class use DBInterface class as member instead of inheritance #74

Merged
merged 1 commit into from
May 27, 2020

Conversation

dzhangalibaba
Copy link
Collaborator

@dzhangalibaba dzhangalibaba commented May 22, 2020

  • issue : Exception seen when trying to connect and database service is not responding #72

  • the root cause is : SonicV2Connector connect(dbname) calls base class method connect(dbid), while when connection failed and retry, base class try to call self. connect() which is onicV2Connector connect(dbname) since name are the same. It should call its own connect(dbid).

  • for multiDB, we use methods from SonicV2Connector instead of DBInterface based on earlier changes

  • we should make sure DBInterface class method uses its own methods inside, hence making DBInterface class as a member of SonicV2Connector instead of inheritance

Signed-off-by: Dong Zhang [email protected]

@dzhangalibaba
Copy link
Collaborator Author

@rajendra-dendukuri @qiluo-msft Please help review

@dzhangalibaba
Copy link
Collaborator Author

Test

When stop all dockers , retry connect until database is restarted and set/get works fine

admin@ASW-7005:~$ cat /etc/sonic/testdb.py 
#!/usr/bin/env python2.7
from swsssdk import SonicV2Connector

db = SonicV2Connector()
db.connect(db.CONFIG_DB,True)

db.set("CONFIG_DB", "hashhash", "kk", "vv")
print(db.get_all("CONFIG_DB", "hashhash"))
admin@ASW-7005:~$ /etc/sonic/testdb.py 
{'kk': 'vv'}
admin@ASW-7005:~$ 


admin@ASW-7005:~$ docker stop $(docker ps -aq)
edfd0e02be03
b1a76602159b
f16090b43b43
9b67c63b2c8f
bb2f217daea0
4026b5e72382
c090c9da180c
7ddce00e4387
23df59d0f92b
a7fc3d31a590
4d142775e9b3
5ee83628e713
48a5d88bdf5e
725d3b951e66
admin@ASW-7005:~$ /etc/sonic/testdb.py 
{'kk': 'vv'}
admin@ASW-7005:~$ 

time.sleep(self.CONNECT_RETRY_WAIT_TIME)
self.connect(db_id, True)
self._connect(db_id, True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_connect [](start = 13, length = 8)

We need to fix SonicV2Connector. There is nothing wrong in DBInterface

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use composition instead and only change SonicV2Connector wrapper.

@qiluo-msft
Copy link
Contributor

qiluo-msft commented May 23, 2020

class SonicV2Connector(DBInterface):

Could it use composition instead of inheritance? #Closed


Refers to: src/swsssdk/dbconnector.py:226 in 4391266. [](commit_id = 4391266, deletion_comment = False)

@dzhangalibaba
Copy link
Collaborator Author

class SonicV2Connector(DBInterface):

Could it use composition instead of inheritance?

Refers to: src/swsssdk/dbconnector.py:226 in ac457fb. [](commit_id = ac457fb, deletion_comment = False)

yes we can use it. Updated

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@dzhangalibaba dzhangalibaba changed the title [MultiDB] rename SonicV2Connector base class methods names [MultiDB] SonicV2Connector class use DBInterface class as member instead of inheritance May 26, 2020
@rajendra-dendukuri
Copy link
Contributor

Verified the patch and it looks good.

@qiluo-msft qiluo-msft merged commit 132f8d5 into sonic-net:master May 27, 2020
@SuvarnaMeenakshi
Copy link
Contributor

@qiluo-msft , @dzhangalibaba is there a plan to merge this to 201911 branch?

@qiluo-msft
Copy link
Contributor

Yes. Added the label.

@dzhangalibaba dzhangalibaba deleted the swsssdkclose branch June 9, 2020 06:41
praveen-li pushed a commit to praveen-li/sonic-py-swsssdk that referenced this pull request May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants