Skip to content
This repository has been archived by the owner on Jan 8, 2019. It is now read-only.

Need to remove unused zookeeper functions #1163

Open
bijugs opened this issue Apr 6, 2018 · 9 comments
Open

Need to remove unused zookeeper functions #1163

bijugs opened this issue Apr 6, 2018 · 9 comments
Assignees

Comments

@bijugs
Copy link
Contributor

bijugs commented Apr 6, 2018

For e.g. here, the code got changed from :path to 'path' which is not recognized by ZooKeeper ruby.

@vt0r
Copy link
Member

vt0r commented Apr 6, 2018

Ouch. Looks like a mistake when trying to move from pre- to post-1.9 hash syntax. Should read like this I think:
path: znode_path, data: node_name

@bijugs
Copy link
Contributor Author

bijugs commented Apr 6, 2018

:path still works, but we have 'path' in quotes which I think never worked. The change went in as part of the commit "cf690ae7f5da6b9cf240b075bbbc01df8704c32a". If we have moved to locking cookbook, assuming some of these functions can be removed.

@leochen4891
Copy link
Contributor

@bijugs Do you have an error message?

@cbaenziger
Copy link
Member

We should have moved to locking_resource now. If we can remove these functions too that'd be great! As I recall formatting the HDFS ZKFC znode may use these znode handling functions though.

@bijugs
Copy link
Contributor Author

bijugs commented Apr 6, 2018

ran zk.create('path' => znode_path, 'data' => node_name) manually on chef-shell and it returns a RC of -8 which corresponds to ZBADARGUMENTS. The chef-bach function returns false as if it is an issue with acquiring the lock i.e. creation of znode.

@leochen4891
Copy link
Contributor

leochen4891 commented Apr 6, 2018

chef (12.19.36)> ret = zk.create('path' => znode_path, 'data' => node_name)
Zookeeper::Exceptions::BadArguments: Supported arguments are: [:path, :data, :acl, :ephemeral, :sequence, :callback, :callback_context], but arguments ["path", "data"] were supplied instead
        from /opt/chef/embedded/lib/ruby/gems/2.3.0/gems/zookeeper-1.4.11/lib/zookeeper/client_methods.rb:267:in `assert_keys'
        from /opt/chef/embedded/lib/ruby/gems/2.3.0/gems/zookeeper-1.4.11/lib/zookeeper/client_methods.rb:90:in `create'
        from (irb):24
        from /opt/chef/embedded/lib/ruby/gems/2.3.0/gems/chef-12.19.36/lib/chef/shell.rb:75:in `block in start'
        from /opt/chef/embedded/lib/ruby/gems/2.3.0/gems/chef-12.19.36/lib/chef/shell.rb:74:in `catch'
        from /opt/chef/embedded/lib/ruby/gems/2.3.0/gems/chef-12.19.36/lib/chef/shell.rb:74:in `start'
        from /opt/chef/embedded/lib/ruby/gems/2.3.0/gems/chef-12.19.36/bin/chef-shell:34:in `<top (required)>'
        from /usr/bin/chef-shell:57:in `load'
        from /usr/bin/chef-shell:57:in `<main>'

path: znode_path, data: node_name

works, thanks Sal

@cbaenziger
Copy link
Member

cbaenziger commented Apr 6, 2018

So nothing should be calling acquire_restart_lock or zk.create in Chef-BACH any more -- that should now all be in locking_restart; that looks to be orphaned code? The only znode related function still called in Chef-BACH should be znode_exists.

@bijugs Where are you seeing zk.create run?

@bijugs
Copy link
Contributor Author

bijugs commented Apr 6, 2018

As I mentioned in my earlier comment, if these functions are not used anymore we should remove them and this will be a non issue. It will prevent from any future use as I was trying to use it.

@cbaenziger cbaenziger changed the title Changes made part of code formatting is breaking Need to remove unused zookeeper functions Apr 6, 2018
@cbaenziger
Copy link
Member

Cool! Thanks @bijugs ; I've updated the title to reflect the desired direction and what is up. Yes, let us remove all Zookeeper related functions out of bcpc

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants