Using the latest dev, I'm importing a large csv file with several thousand lines. However, rather than going through all the lines, it keeps on importing the first 5 dozen lines or so over and over.

Is this because I don't have a GUID set?

Thanks!

Comments

alex_b’s picture

Category: support » bug

I assume you import nodes.

Unique targets (I assume this is what you mean by GUIDs) aren't required for batching to work. When a processor (e. g. FeedsNodeProcessor) processes an import batch (FeedsImportBatch), it actually removes each item it has processed. When there are more items than what can be processed in one page load (FEEDS_NODE_BATCH_SIZE) the FeedsImportBatch is saved and Batch API is informed that the batch hasn't finished yet.

Here is where I would start debugging:

FeedsSource::import()

public function import() {
    try {
      if (!$this->batch || !($this->batch instanceof FeedsImportBatch)) {
        $this->batch = $this->importer->fetcher->fetch($this);
        $this->importer->parser->parse($this->batch, $this);
      }
      $result = $this->importer->processor->process($this->batch, $this);
      if ($result == FEEDS_BATCH_COMPLETE) {
        unset($this->batch);
        module_invoke_all('feeds_after_import', $this->importer, $this);
      }
    }
    catch (Exception $e) {
      unset($this->batch);
      $this->save();
      throw $e;
    }
    $this->save();
    return $result;
  }

For some reason $this->batch in the first if statement may not be set in the second call of the batch process, while it should be. For some reason $this may not have been properly saved in the first call of the batch process (see the lines $this->save() in the same method).

If you are not able to debug this issue yourself, could you post the (anonymized) CSV file that causes this problem and describe exactly your system configuration (esp. PHP version) and feeds importer configuration?

ManyNancy’s picture

Thank you for all your help.

I i don't know how to debug this on my own, so some info:

I'm running (mercury0.9) apache2 mod_php, PHP Version 5.2.6-3ubuntu4.5, pressflow, cacherouter with APC.

My import config is a clone of node import with a CT set (so that the nodes all become one ct):

* Attached to: [none]
* Refresh: never
* Import on create

File upload
Upload content from a local file.

CSV parser
Parse data in Comma Separated Value format.

Node processor
Create nodes.

I unchecked the unique target for the GUID

my csv file is like this:

title
node1title
node2title
node3title

... etc. No quotation marks.

Edit:

Also tried it with quotes:
title
"node1title"
"node2title"
"node3title"

The problem still exists.

Thanks!

jerdavis’s picture

(copied from #600584)

I'm seeing some really odd behavior with this and I'm having a hard time debugging. To be fair, I need to go back and test this with stock feeds before I can really open an issue, but I wanted to get just a general sense on a couple of things I'm seeing.

In this use case I have a 700 line CSV file. This file is being imported twice by two different stand alone feeds (no feed node) to populate 2 different content types.

In the first case each line will result in a node creation as each line is unique. This is to create user profiles containing a subset of the CSV data and using a GUID unique to each line. This import works fine through the batch API implementation, however the progress bar does not update as the properties of $batch are not being updated. At the end of the import it says that 50 nodes were created, although it actually created 700.

In the second case we're using a different set of columns from the CSV to create a related organization. Each line contains information about the organization the member belongs to, however many of the members belong to the same organizations so rather than 700 nodes created we create 150. This is using a different column as the unique value. While not really efficient, it works. Or it did before the batch API implementation.

This second case behaves very oddly. The import starts and the batch is initialized. A small set, usually 10 nodes is created. Then those same 10 nodes are updated continually and it never moves past to the rest. I think this may be loosely tied to the issue with the properties of $batch not being updated well in the previous example, but with more severe consequences in that the import gets thrown into an infinite loop and fails.

So, right now my point is how fully tested is this batch API implementation? Does anyone have thoughts on what is preventing $batch from being updated? I did notice that if I added the public properties of FeedsBatch to FeedsImportBatch that things worked a bit better (I could now see those values in the serialized column in feeds_source table). However it did not solve the other issues. I'm wondering if part of the issue might go back to the fact that $batch contains $batch->items, which may be causing a problem with the serialization?

jerdavis’s picture

Status: Active » Needs review

Some progress here (potentially).

There are a few problems that I'm seeing here, and I'm unsure the best approach to deal with them. It seems PHP has some issues with serializing objects with protected properties. This is causing part of the problem, namely that when the import runs it tries to save $this->batch to feeds_source, however what gets populated to the database is incomplete due to the serialization/unserialization issues with protected properties. Thus the FeedsSource::load() function never replaces $this->batch with the saved value. Thus the progress bar and created/updated counts never increment fully.

I "fixed" this by doing two things, I set all of the properties for FeedsImportBatch and FeedsFileFetch to public. It was still having a problem serializing $this->batch->items for storage, so I also added base64_encode() and base64_decode() to FeedsSource::save(). and FeedsSource::load() respectively.

This resulted in my first import working successfully, complete with updating progress bar (yay!).

I'm still having the problem with my second case however. I'm going to need to do a bit more troubleshooting here.

Let me know what you think of these approaches and any pros/cons you can think of.

jerdavis’s picture

Hackish changes for reference...


  /**
   * Save configuration.
   */
  public function save() {
    $config = $this->getConfig();
    // Alert implementers of FeedsSourceInterface to the fact that we're saving.
    foreach ($this->importer->plugin_types as $type) {
      $this->importer->$type->sourceSave($this);
    }
    // Store the source property of the fetcher in a separate column so that we
    // can do fast lookups on it.
    $source = '';
    if (isset($config[get_class($this->importer->fetcher)]['source'])) {
      $source = $config[get_class($this->importer->fetcher)]['source'];
    }
    $batch = isset($this->batch) ? $this->batch : FALSE;

    $object = array(
      'id' => $this->id,
      'feed_nid' => $this->feed_nid,
      'config' => $config,
      'source' => $source,
      'batch' => base64_encode(serialize($batch)), //isset($this->batch) ? $this->batch : FALSE,
    );
    // Make sure a source record is present at all time, try to update first,
    // then insert.
    drupal_write_record('feeds_source', $object, array('id', 'feed_nid'));
    if (!db_affected_rows()) {
      drupal_write_record('feeds_source', $object);
    }
  }

  /**
   * Load configuration and unpack.
   *
   * @todo Patch CTools to move constants from export.inc to ctools.module.
   */
  public function load() {
    if ($record = db_fetch_object(db_query('SELECT config, batch FROM {feeds_source} WHERE id = "%s" AND feed_nid = %d', $this->id, $this->feed_nid))) {
      // While FeedsSource cannot be exported, we still use CTool's export.inc
      // export definitions.
      ctools_include('export');
      $this->export_type = EXPORT_IN_DATABASE;
      $this->config = unserialize($record->config);
      $this->batch = unserialize(base64_decode(unserialize($record->batch)));
    }
  }
jerdavis’s picture

OK, I take part of that back. I had another change in place which was executing the parser on each batch, once I took that change out both imports are working dandy.

I can roll a patch if you feel these changes are acceptable. If you had good reasons for setting the properties to protected we may need to revisit.

alex_b’s picture

I "fixed" this by doing two things, I set all of the properties for FeedsImportBatch and FeedsFileFetch to public. It was still having a problem serializing $this->batch->items for storage, so I also added base64_encode() and base64_decode() to FeedsSource::save(). and FeedsSource::load() respectively.

*wince* - I did not see this one coming.

Does the base64 coding work reliably? Should we serialize the entire batch object using base64 encoding? I see you use it for making sure that the items array is serialized properly (indeed we have very little control of what's in the items array).

If you had good reasons for setting the properties to protected we may need to revisit.

My only issue here is that why would we set FeedsImportBatch's (and its extension's) properties to public when we need to make sure that possibly protected/private members in the items array need to be serialized/unserialized cleanly?

Does base64 encoding solve the problem with private members at all?

jerdavis’s picture

StatusFileSize
new1.67 KB

Patch against CVS HEAD - working on all imports here. I also added the $processed property and included it in FeedsNodeProcessor.inc.

I'm a little concerned about $batch->items being stored in the database like it is. This seems like it could become a problem when processing a large number of records.

One thought would be to add a method to FeedsImportBatch that pulls a chunk of data from the array based on the state of the counters, basically implement a db_query_range() equivalent using array_slice(). This would mean you'd have to re-parse the file each iteration to get something to slice - I don't know which would be worse. Schlepping around a giant base64 encoded serialized array in and out of the database every run, or parsing the source each run to have something to slice from.

alex_b’s picture

StatusFileSize
new3.79 KB

#8: thank you for the patch. I did a slight cleanup, notably I have removed the 'serialized' flag from the batch field which does away with the double unserialize() on load().

I have committed a modification to the test "Defaults: Node import". It now imports a 79 line CSV file for testing.

The only thing I struggle with is I can't reproduce the error reported here. While the base64 encoding works fine, I can't find out what it actually fixes.

I am using the default importer called "Node import" (part of Feeds Defaults module) and I import the CSV file in tests/feeds/many_nodes.csv on PHP5.2.6 and MySQL 5.0.77 . I have tried the importer with and without a unique target setting and it works fine.

ManyNancy: what's your MySQL version?
jerdavis and ManyNancy: does the import of many_nodes.csv with "Node import" fail on your system?

alex_b’s picture

Title: Batch import does not continue where it left off, instead starts from the beginning! » Batch import does not continue where it left off, instead starts from the beginning
Priority: Normal » Critical

This fix is release critical.

rjbrown99’s picture

I implemented this patch on the latest cvs release as of last night. I did a 12,000 node import from an XML file and can report I had no problems.

I never had the original problem, but I can at least say this didn't break anything.

The progress bar still did not move for me, though. It stayed at 0 until it completed processing.

Grayside’s picture

I applied this patch, then noticed my HTTP-fetched CSV was capped at 30 items out of 400+, so I switched to an uploaded CSV file. Imported several hundred items.

Ran it twice, first time it said ~59 nodes created, second time "Created 38 Book page nodes." This is wrong, the 400+ nodes were created.

Why the HTTP-fetched CSV file was persistently limited to 30 nodes I'm not sure. Possibly the multi-line spacing in the node body was choking something.

rjbrown99’s picture

The patch in #9 now fails to apply cleanly against CVS HEAD and Alpha 14. Specifically, the structure of the feeds.install file function feeds_update_6009() has gone in a different direction as of file version 1.7.

I just dropped the relevant part of the code into the function near the end above the return.

$spec = array(
  'type' => 'text',
  'size' => 'big',
  'not null' => FALSE,
  'description' => t('Cache for batching.'),
  'serialize' => TRUE,
);
db_change_field($ret, 'feeds_source', 'batch', 'batch', $spec);
alex_b’s picture

Status: Needs review » Needs work

NW then.

rjbrown99’s picture

Status: Needs work » Needs review
StatusFileSize
new3.66 KB

Here's a re-roll of the patch against HEAD, which at the moment should be the same as alpha15. I'm changing it back to NR since it now applies cleanly. I did not make any other changes or enhancements.

alex_b’s picture

@rjbrown99: Can you confirm any of the abovementioned problems occurring when not having this patch applied?

rjbrown99’s picture

Well, I'm having a few issues at the moment and I'm not sure what they are related to yet. This was all with alpha14 - I just updated to 15 last night and haven't re-imported yet.

What happens with me is the import seems to get hung up at different points. I'll have a small import (around 300 product/items) and it will get to 99% on the display but will have actually finished all of the items in the import. With a larger import (on the order of 5000 product/items) I may get 25% of the way complete and then it hangs up. When I say hang I mean Feeds stops the progress bar and dumps me the Feeds error message. There are no relevant things in the httpd logs or the drupal logs. So my issue is a little different because I'm not seeing it re-import the same items over and over, but I am seeing it stop/hang. This patch is included in my build because I generally have to re-start the feeds import process a few times after it gets hung up.

I haven't reported issues yet because I haven't tracked down what might be causing this. My case is a bit more complex because I am using Feeds with all of the following enhancements/patches:
#623444: Port FeedAPI Mappers: link, patch in comment 44
#637334: Content Taxonomy Mapper, patch in comment 14
#652180: Assign author of imported nodes, patch in comment 23
#744660: Expand batch support to fetchers and parsers, patch in comment 27
This issue, patch in comment 15

Plus a few of my own:
#688696: NodeProcessor: Ignore changes to certain fields upon node update
#706908: Enhance filefield mapper to perform more validation on remote URLs

... and because I like to make life extra difficult for myself, I also wrote my own XML parser and processor (which is just an extension of the process class from FeedsNodeProcessor). So things that I encounter may or may not be the same as what others see.

tmcw’s picture

I was getting the same kind of behavior, and even worse - only 50 nodes were deleted by a node processor-based feed importer at a time, and all imports would stall after 50. Applying this patch has eliminated that and all related problems. I'm using PHP 5.3.1 and MySQL client API mysqlnd 5.0.5-dev, and MySQL 5.1.47.

fereira’s picture

I don't know if this is related to the issue here but I have had problems with the Feeds module when trying to delete nodes create by an importer. When I tried to delete all of the nodes the progress bar would show that it was initializing, then show 0%, then just hang. Eventually it would display a memory exhausted error. I noticed in the latest README file that a list of "hidden settings" was provided and that "feeds_node_batch_size" was set to 50. I reduced it to 10 and the delete now works. In this case it was deleting 40 nodes (and all 40 deleted).

rjbrown99’s picture

Not sure why, but I have the following problem with #15 related to batching and the creation of nodes.

1) FeedsNodeProcessor::process() starts.
2) $batch->items begins with every item from 0 through 11123 (the number of items in this import)
3) The initial process() takes place on items 0 through 1684 (based on lots of items not needing an update)
4) The process() function restarts as a continued batch
5) This time, $batch->items only has 50 items
6) We process the 50 items, and then exit cleanly

The problem is that on the second run, $batch only has those 50 items so it thinks we are done. It should have the entire list of items from 0 through 11123, and then pick up processing at the offset.

Backing out this change seems to have fixed that issue. I have not investigated why this is happening but for now I'm not going to be using this patch.

rbayliss’s picture

StatusFileSize
new419 bytes

I'm not sure if this is the same problem, but I was having an issue with hanging batch imports for CSV HTTP fetches. Some debugging pointed to encoding problems in the serializing/unserializing process used to track batch progress in the database. The following patch appears to have fixed the issue. Again, I'm not entirely sure this is related to the above posts, but it appears to have fixed my problem.

vunger’s picture

I'm having the same problem with 6.x-1.0-beta7.

derhasi’s picture

Issue tags: +batch, +serialize
StatusFileSize
new2.22 KB

I had the same problem, when importing Excel files with more than 50 lines (using feeds_excel). It stuck at 1% or more - dependent on local machine or server.

So I modified the patch in #15 to fit the current dev:
* renaming update function
* removing ->processed handling, as #849986: Cleaner batch support allready successfully dealt with this

This (patch attached) now works for me. So I'd appreciate some review, so this could finally get rolled out.

alex_b’s picture

So I'd appreciate some review, so this could finally get rolled out.

It's been very hard to get good reviews for this patch because only few people run into this problem and the base64encode workaround seems - er - very much like witchcraft...

FWIW, the 'real' solution is to avoid stashing parsed items in the DB like Feeds 7.x 2.x does that - see #744660: Expand batch support to fetchers and parsers.

I think you'll have to run with a patched Feeds version for a while. Drush make can be of great help for that as it allows for applying patches automatically to projects in your make file manifest.

http://drupal.org/project/drush_make

derhasi’s picture

StatusFileSize
new1.92 KB

I totally agree.

I got another idea that could be added to FeedsImportBatch and looked after in FeedsSource::import().
This solution gives the opportunity for a parser to add new items, after a given amount of items has been processed by setting a "partial" state.
If the new $batch->partial is not set, behaviour would be as is.

One use case could be for cvsParser, that will process 100 items per run. In every run, the parser only loads 100 lines, and throws them into the batch. After the batch queue got processed, the parser could set the next 100 items.

Another example that could be applied for my feeds_excel plugin's ExcelParser, by only processing some items (in this example 10), avoid storing the whole excel file, and then reload the file in the next step.
Yes this will cause a lot of memory usage (especially for only 10 items processing), but this might be better than trying to store a too big file in the database.

  public function parse(FeedsImportBatch $batch, FeedsSource $source) {
    $filepath = $batch->getFilePath();
    $data = new Spreadsheet_Excel_Reader();
    $data->read($filepath);
    $items = $this->getItems($sheets);

    $c = count($items);
    if ($c > 10) {
      if ($batch->isPartial()) {
        $params = $batch->partial['params'];
        $first = $params['first'];
      }
      else {
        $first = 0;
      }
      $factor = ($c < $first + 10) ? 1 : ($first + 10) / $c;
      $items = array_slice($items, $first, 10);
      $batch->setPartial($factor, array('first' => $first + 10));
    }
    elseif ($batch->isPartial()) {
      $batch->unsetPartial();
      $items = array();
    }
    $batch->setItems($items);
  }
derhasi’s picture

StatusFileSize
new3.5 KB

A more readable patch file:

alex_b’s picture

#25: this is essentially how full batching works in 7.x 2.x. I'd rather backport functionality than creating two diverging approaches for the same functionality :) If you're interesting in backporting full batching to 7.x 2.x, let's talk (I'm on IRC). It's going to take a major version jump for the D6 version, but it's doable.

customweb’s picture

For people, who has the also the problem that the progress bar hangs and the system creates more and more nodes. The problem can be that you do not have used the right encoding (UTF-8). This can cause similar problems like the above posters describes.

caccamo’s picture

Hi, can you be more specific with regards to select the right encoding? I don't see the setting anywhere (or are you saying that the original CSV file must be saved in UTF8?)

customweb’s picture

Hi caccamo,
You can only use UTF8 encoded CSV files. This is probably somewhere documented, i do not know that at the moment.

Error happens when you use a CSV file that was not encoded with UTF8. Microsoft Excel tends to do this wrong.

kehan’s picture

I'm having the same issue in feeds 7.x-2.x-dev, but obviously the above patches won't apply. Is there any work around for 7.x?
Thanks,
K
(Willing to test / help roll patches)

404’s picture

subscribe

similiar problem on 7.x

csv file, one record per line.

after importing dozens of nodes, feeds messed up the record: taking more lines as a column

derhasi’s picture

Stumbled upon #1139376: Batch processing fails on large feeds where Creel seems to have another solution for this issue, I guess.

twistor’s picture

StatusFileSize
new860 bytes

#1139376: Batch processing fails on large feeds is only tangentially related to this bug. I'm still not quite sure why this bug exists, but I have a much more straightforward solution. Change the text:big to blob:big. Turning on strict mode in mysql points to an error in the query. That makes me think it's a mysql bug.

This probably should have been blob from the beginning anyway, as we don't ever query against the field. I will investigate if this error exists in 7.x now that I am able to reproduce it.

twistor’s picture

Har! #690746: Text column type doesn't reliably hold serialized variables. To summarize, when serialize() is called on an object with protected, or private, members null characters get thrown in which also truncate the string. The issue claims that postgres and sqlite are the only ones affected. Either way, that looks like a reasonable explanation of what's happening here.

twistor’s picture

StatusFileSize
new869 bytes

Updated the comment on hook_update.

masseuro’s picture

#36 fix for me !

Great thx twistor

twistor’s picture

Version: 6.x-1.x-dev » 7.x-2.x-dev
StatusFileSize
new1.47 KB

Bumping to 7.x as it's a problem there as well.

joeredhat-at-yahoo.com’s picture

I'm using the patch from comment 38 and it has solved the problem for me.

twistor’s picture

tagging

Timusan’s picture

I applied the patch #38 and I can confirm it works correctly.

Before the patch I could not import my csv containing 280 lines, it failed at 50 or 100 lines.
After the patch all 280 products (using feeds for Commerce) were imported without any problems.
I too use PostgreSQL.

stackpr’s picture

#38 worked for me on 6.x-1.x.

I use MySQL, and the bug appeared after converting the table from MyISAM to InnoDB. I had not run the import for a few weeks prior to the table type conversion, so that may have been a coincidence. Thanks for the patch!

twistor’s picture

Status: Needs review » Reviewed & tested by the community

Got one more testimonial from http://drupal.org/node/1365792#comment-5753332.

Gonna RTBC this.

twistor’s picture

Status: Reviewed & tested by the community » Patch (to be ported)
twistor’s picture

Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

twistor’s picture

Removing tag.