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

Feature Request: make SitemapGenerator::MAX_SITEMAP_LINKS a configuration...? #188

Closed
PikachuEXE opened this issue Jan 15, 2015 · 13 comments

Comments

@PikachuEXE
Copy link

I understand the documented number of links limit per sitemap file is 50,000.
But a SEO team we hired recommends 5k:

You'll want to look into some kind of server-side plugin that can generate sitemap indexes since XML sitemaps should have less than 5k URLs each, but ideally 1-2k

I am not sure if there are enough people needing this, hence putting the question mark.

@PikachuEXE
Copy link
Author

I currently just assign to the already defined constant SitemapGenerator::MAX_SITEMAP_LINKS

@kjvarga
Copy link
Owner

kjvarga commented May 13, 2015

That's the right way to do it until this becomes an option. Would be useful I agree.

@PikachuEXE
Copy link
Author

Need a PR?

@kjvarga
Copy link
Owner

kjvarga commented May 14, 2015

Yes please

On Wednesday, May 13, 2015, PikachuEXE notifications@github.com wrote:

Need a PR?


Reply to this email directly or view it on GitHub
#188 (comment)
.

Karl Varga
e: kjvarga@gmail.com
w: kjvarga.blogspot.com
c: +1.250.884.9895
Skype: kjvarga

@PikachuEXE
Copy link
Author

Where to put it?

  • SitemapGenerator::Sitemap
  • SitemapGenerator
  • SitemapGenerator.configuration
  • SitemapGenerator::Sitemap.configuration
    How to name the config?
  • max_sitemap_files

@kjvarga
Copy link
Owner

kjvarga commented May 20, 2015

So how I want to do this is to add :max_sitemap_files as an option to SitemapGenerator::LinkSet class (including attribute accessors). When that initializes the SitemapGenerator::Builder::SitemapIndexFile in the sitemap_index function, pass the option in to the initializer. Then have the index file use the option and not the global constant.

Groups share the index file, so this way, with multiple groups in a sitemap, the number of files should be constrained across all groups. I'd want to sanity check that things work correctly when SitemapGenerator::LinkSet :create_index option is false. AFAI can remember we create an index file regardless, but just don't write it out, so things should still work, but I'd want to confirm that.

@PikachuEXE
Copy link
Author

I am sure I was talking about max_sitemap_links only, not max_sitemap_files
So #198 is opened for max_sitemap_links

@kjvarga
Copy link
Owner

kjvarga commented May 20, 2015

Ah I got confused by the title of this issue.

@PikachuEXE
Copy link
Author

Oops let me update the title

@PikachuEXE PikachuEXE changed the title Feature Request: make SitemapGenerator::MAX_SITEMAP_FILES a configuration...? Feature Request: make SitemapGenerator::MAX_SITEMAP_LINKS a configuration...? May 21, 2015
@chrisrichard
Copy link

Any chance lowering the max_sitemap_links setting will limit memory use when generating sitemaps with a lot of links? Does the generator write the sitemap files to disk as soon as the limit is reached or is everything kept in memory and written out at the end?

@kjvarga
Copy link
Owner

kjvarga commented Feb 6, 2017

@chrisrichard it could help reduce memory use, but I suspect your problem is elsewhere, like how you are loading your ActiveRecord records for example. Each sitemap file is held in memory until it is written to disk, then the memory is freed. The index file has to stay in memory until all sitemaps have been written. If you have lots of AR records, you probably want to use a batched find, cause you might be loading all your records into memory before you start writing the sitemaps.

@kjvarga
Copy link
Owner

kjvarga commented Feb 17, 2017

This is almost ready to release #262. I ended up doing a heck of a lot of cleanup in the specs and some other housecleaning so I'm being cautious and testing. Will release it very soon.

@kjvarga
Copy link
Owner

kjvarga commented Feb 21, 2017

Released in v5.3.0

@kjvarga kjvarga closed this as completed Feb 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants