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

Module cache improvements #9220

Merged
merged 9 commits into from
Jan 18, 2018
Merged

Conversation

clee-r7
Copy link
Contributor

@clee-r7 clee-r7 commented Nov 17, 2017

As part of the new data service changes (aka: Goliath) clean up of the current module caching was needed to allow an easier port to a data services model.

Changes:

  1. Framework now ships with a baseline metadata module file under the "db" directory. While not necessary for startup it enhances the first time user experience by not needing all the modules to first be loaded into memory when running without a DB.
  2. Whether running with or without a DB an on disk representation of the module metadata is now stored to disk: //store/. This file is generated from the base file mentioned above and may also include user specific modules.

Side effects:
When running without a DB:

  1. Memory usage on startup was approximately 70-100mb less
  2. Startup time was notably faster
  3. Module search now in the ms range not the ~15+ sec range when tested

When running with a DB

  1. Changes are negligible

Copy link
Contributor

@sempervictus sempervictus left a comment

Choose a reason for hiding this comment

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

Neat, thank you. Looks sane at first pass, will try to get this merged ob my end and test it out.

@@ -21,7 +21,7 @@ module ClassMethods
#

def fullname
type + '/' + refname
type + '/' + (refname.nil? ? '' : refname)
Copy link
Contributor

Choose a reason for hiding this comment

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

"#{type}/#{refname}" maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you

@@ -98,7 +99,7 @@ def load_cached_module(type, reference_name)
end

# @overload refresh_cache_from_module_files
# Rebuilds database and in-memory cache for all modules.
# Rebuilds module metadat store and in-memory cache for all modules.
Copy link
Contributor

Choose a reason for hiding this comment

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

metadata

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you

@jmartin-tech
Copy link
Contributor

The specs failing in ruby 2.3 look like they are things that should either be updated or removed.

Failed examples:

rspec './spec/lib/msf/core/module_manager_spec.rb[1:1:8:2:1]' # Msf::ModuleManager it should behave like Msf::ModuleManager::Cache #module_info_by_path_from_database! with database cache should create cache entry for path
rspec './spec/lib/msf/core/module_manager_spec.rb[1:1:8:2:2:3]' # Msf::ModuleManager it should behave like Msf::ModuleManager::Cache #module_info_by_path_from_database! with database cache cache entry
rspec './spec/lib/msf/core/module_manager_spec.rb[1:1:8:2:2:4]' # Msf::ModuleManager it should behave like Msf::ModuleManager::Cache #module_info_by_path_from_database! with database cache cache entry
rspec './spec/lib/msf/core/module_manager_spec.rb[1:1:8:2:2:1]' # Msf::ModuleManager it should behave like Msf::ModuleManager::Cache #module_info_by_path_from_database! with database cache cache entry
rspec './spec/lib/msf/core/module_manager_spec.rb[1:1:8:2:2:2]' # Msf::ModuleManager it should behave like Msf::ModuleManager::Cache #module_info_by_path_from_database! with database cache cache entry
rspec './spec/lib/msf/core/module_manager_spec.rb[1:1:8:2:3:2:1]' # Msf::ModuleManager it should behave like Msf::ModuleManager::Cache #module_info_by_path_from_database! with database cache typed module set without reference_name should set reference_name value to Msf::SymbolicModule

@clee-r7
Copy link
Contributor Author

clee-r7 commented Nov 20, 2017

My last commit has a one line that shouldn't have caused the tests to be failing. How do I kick off the tests again (without recommit)?

@busterb
Copy link
Member

busterb commented Nov 20, 2017

Click 'Details' on the Travis CI build failed link above. Make sure you login to Travis CI with your github credentials first, you'll have a 'Restart build' button available then.

@jmartin-tech
Copy link
Contributor

jmartin-tech commented Nov 20, 2017

I restarted test again, I expect may still fail.

For me the tests fail on 2.3.5 but not on 2.4.2 and I ran locally on Linux to validate.

@jmartin-tech
Copy link
Contributor

I would suggest adding 'db/modules_metadata_base.pstore' to .gitignore. While it should be updated at times this would enforce explicit add of the file so user local data does not have end up getting added/leaked into master. We can add periodic update to the weekly release process to keep this from becoming a requirement to update each time a module changes or is added.

@clee-r7
Copy link
Contributor Author

clee-r7 commented Nov 21, 2017

I kicked off the builds this morning and they are all passing now :/

@jmartin-tech
Copy link
Contributor

@chlee: "I kicked off the builds this morning and they are all passing now :/"

That makes me sad, I validated externally for the failed tests, wonder why it would start passing in travis now :-(

Now looks like it is having a failure in 2.4.2, I suspect this is timing related, some tests are expecting caching to complete before run. We need to get consistent results before we land this.

@clee-r7
Copy link
Contributor Author

clee-r7 commented Nov 21, 2017

@jmartin-r7 - I think I see the problem - despise timing tests

@busterb busterb self-assigned this Jan 18, 2018
@busterb busterb merged commit e0d8f8e into rapid7:master Jan 18, 2018
busterb added a commit that referenced this pull request Jan 18, 2018
@busterb
Copy link
Member

busterb commented Jan 18, 2018

Landed, as the first Metasploit 5 PR. Thanks @clee-r7!

@busterb
Copy link
Member

busterb commented Jan 18, 2018

Added 86c927e to fix a problem with msfvenom running.

jmartin-tech pushed a commit to jmartin-tech/metasploit-framework that referenced this pull request Oct 24, 2018
@busterb
Copy link
Member

busterb commented Oct 26, 2018

Release Notes

The module search system has been overhauled to require less memory, return results faster, and to not require a database to work at full speed. New module metadata and search options are also included in this back-ported feature from Metasploit 5.

@gdavidson-r7 gdavidson-r7 added the rn-enhancement release notes enhancement label Nov 4, 2018
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.

None yet

8 participants